Re: [PATCH V7 14/15] PCI: tegra: Add Tegra194 PCIe support

From: Thierry Reding
Date: Tue May 21 2019 - 07:44:14 EST


On Fri, May 17, 2019 at 06:08:45PM +0530, Vidya Sagar wrote:
> Add support for Synopsys DesignWare core IP based PCIe host controller
> present in Tegra194 SoC.
>
> Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>
> ---
> Changes since [v6]:
> * Removed code around "nvidia,disable-aspm-states" DT property
> * Refactored code to remove code duplication
>
> Changes since [v5]:
> * Addressed review comments from Thierry
>
> Changes since [v4]:
> * None
>
> Changes since [v3]:
> * None
>
> Changes since [v2]:
> * Changed 'nvidia,init-speed' to 'nvidia,init-link-speed'
> * Changed 'nvidia,pex-wake' to 'nvidia,wake-gpios'
> * Removed .runtime_suspend() & .runtime_resume() implementations
>
> Changes since [v1]:
> * Made CONFIG_PCIE_TEGRA194 as 'm' by default from its previous 'y' state
> * Modified code as per changes made to DT documentation
> * Refactored code to address Bjorn & Thierry's review comments
> * Added goto to avoid recursion in tegra_pcie_dw_host_init() API
> * Merged .scan_bus() of dw_pcie_host_ops implementation to tegra_pcie_dw_host_init() API
>
> drivers/pci/controller/dwc/Kconfig | 10 +
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pcie-tegra194.c | 1620 ++++++++++++++++++++
> 3 files changed, 1631 insertions(+)
> create mode 100644 drivers/pci/controller/dwc/pcie-tegra194.c
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index a6ce1ee51b4c..c5d71d5d26a0 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -220,6 +220,16 @@ config PCI_MESON
> and therefore the driver re-uses the DesignWare core functions to
> implement the driver.
>
> +config PCIE_TEGRA194
> + tristate "NVIDIA Tegra194 (and later) PCIe controller"
> + depends on (TEGRA_BPMP && ARCH_TEGRA) || COMPILE_TEST
> + depends on PCI_MSI_IRQ_DOMAIN
> + select PCIE_DW_HOST
> + select PHY_TEGRA194_PCIE_P2U
> + help
> + Say Y here if you want support for DesignWare core based PCIe host
> + controller found in NVIDIA Tegra194 SoC.
> +
> config PCIE_UNIPHIER
> bool "Socionext UniPhier PCIe controllers"
> depends on ARCH_UNIPHIER || COMPILE_TEST
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index b085dfd4fab7..b30336181d46 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o
> obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o
> obj-$(CONFIG_PCI_MESON) += pci-meson.o
> +obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
> obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
>
> # The following drivers are for devices that use the generic ACPI
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> new file mode 100644
> index 000000000000..4b3a1d4125b6
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -0,0 +1,1620 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * PCIe host controller driver for Tegra194 SoC
> + *
> + * Copyright (C) 2019 NVIDIA Corporation.
> + *
> + * Author: Vidya Sagar <vidyas@xxxxxxxxxx>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/debugfs.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/interrupt.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/kfifo.h>
> +#include <linux/kthread.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_pci.h>
> +#include <linux/pci.h>
> +#include <linux/pci-aspm.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/random.h>
> +#include <linux/reset.h>
> +#include <linux/resource.h>
> +#include <linux/types.h>
> +#include "pcie-designware.h"
> +#include <soc/tegra/bpmp.h>
> +#include <soc/tegra/bpmp-abi.h>
> +#include "../../pci.h"
> +
> +#define CTRL_5 5

There isn't really a need for this since you only use it once and the
CTRL_ prefix doesn't really add any contextual information in that case.
Might as well just hardcode that 5 in the one location.

> +
> +#define APPL_PINMUX 0x0
> +#define APPL_PINMUX_PEX_RST BIT(0)
> +#define APPL_PINMUX_CLKREQ_OVERRIDE_EN BIT(2)
> +#define APPL_PINMUX_CLKREQ_OVERRIDE BIT(3)
> +#define APPL_PINMUX_CLK_OUTPUT_IN_OVERRIDE_EN BIT(4)
> +#define APPL_PINMUX_CLK_OUTPUT_IN_OVERRIDE BIT(5)
> +#define APPL_PINMUX_CLKREQ_OUT_OVRD_EN BIT(9)
> +#define APPL_PINMUX_CLKREQ_OUT_OVRD BIT(10)
> +
> +#define APPL_CTRL 0x4
> +#define APPL_CTRL_SYS_PRE_DET_STATE BIT(6)
> +#define APPL_CTRL_LTSSM_EN BIT(7)
> +#define APPL_CTRL_HW_HOT_RST_EN BIT(20)
> +#define APPL_CTRL_HW_HOT_RST_MODE_MASK GENMASK(1, 0)
> +#define APPL_CTRL_HW_HOT_RST_MODE_SHIFT 22
> +#define APPL_CTRL_HW_HOT_RST_MODE_IMDT_RST 0x1
> +
> +#define APPL_INTR_EN_L0_0 0x8
> +#define APPL_INTR_EN_L0_0_LINK_STATE_INT_EN BIT(0)
> +#define APPL_INTR_EN_L0_0_MSI_RCV_INT_EN BIT(4)
> +#define APPL_INTR_EN_L0_0_INT_INT_EN BIT(8)
> +#define APPL_INTR_EN_L0_0_CDM_REG_CHK_INT_EN BIT(19)
> +#define APPL_INTR_EN_L0_0_SYS_INTR_EN BIT(30)
> +#define APPL_INTR_EN_L0_0_SYS_MSI_INTR_EN BIT(31)
> +
> +#define APPL_INTR_STATUS_L0 0xC
> +#define APPL_INTR_STATUS_L0_LINK_STATE_INT BIT(0)
> +#define APPL_INTR_STATUS_L0_INT_INT BIT(8)
> +#define APPL_INTR_STATUS_L0_CDM_REG_CHK_INT BIT(18)
> +
> +#define APPL_INTR_EN_L1_0_0 0x1C
> +#define APPL_INTR_EN_L1_0_0_LINK_REQ_RST_NOT_INT_EN BIT(1)
> +
> +#define APPL_INTR_STATUS_L1_0_0 0x20
> +#define APPL_INTR_STATUS_L1_0_0_LINK_REQ_RST_NOT_CHGED BIT(1)
> +
> +#define APPL_INTR_STATUS_L1_1 0x2C
> +#define APPL_INTR_STATUS_L1_2 0x30
> +#define APPL_INTR_STATUS_L1_3 0x34
> +#define APPL_INTR_STATUS_L1_6 0x3C
> +#define APPL_INTR_STATUS_L1_7 0x40
> +
> +#define APPL_INTR_EN_L1_8_0 0x44
> +#define APPL_INTR_EN_L1_8_BW_MGT_INT_EN BIT(2)
> +#define APPL_INTR_EN_L1_8_AUTO_BW_INT_EN BIT(3)
> +#define APPL_INTR_EN_L1_8_INTX_EN BIT(11)
> +#define APPL_INTR_EN_L1_8_AER_INT_EN BIT(15)
> +
> +#define APPL_INTR_STATUS_L1_8_0 0x4C
> +#define APPL_INTR_STATUS_L1_8_0_EDMA_INT_MASK GENMASK(11, 6)
> +#define APPL_INTR_STATUS_L1_8_0_BW_MGT_INT_STS BIT(2)
> +#define APPL_INTR_STATUS_L1_8_0_AUTO_BW_INT_STS BIT(3)
> +
> +#define APPL_INTR_STATUS_L1_9 0x54
> +#define APPL_INTR_STATUS_L1_10 0x58
> +#define APPL_INTR_STATUS_L1_11 0x64
> +#define APPL_INTR_STATUS_L1_13 0x74
> +#define APPL_INTR_STATUS_L1_14 0x78
> +#define APPL_INTR_STATUS_L1_15 0x7C
> +#define APPL_INTR_STATUS_L1_17 0x88
> +
> +#define APPL_INTR_EN_L1_18 0x90
> +#define APPL_INTR_EN_L1_18_CDM_REG_CHK_CMPLT BIT(2)
> +#define APPL_INTR_EN_L1_18_CDM_REG_CHK_CMP_ERR BIT(1)
> +#define APPL_INTR_EN_L1_18_CDM_REG_CHK_LOGIC_ERR BIT(0)
> +
> +#define APPL_INTR_STATUS_L1_18 0x94
> +#define APPL_INTR_STATUS_L1_18_CDM_REG_CHK_CMPLT BIT(2)
> +#define APPL_INTR_STATUS_L1_18_CDM_REG_CHK_CMP_ERR BIT(1)
> +#define APPL_INTR_STATUS_L1_18_CDM_REG_CHK_LOGIC_ERR BIT(0)
> +
> +#define APPL_MSI_CTRL_2 0xB0
> +
> +#define APPL_LTR_MSG_1 0xC4
> +#define LTR_MSG_REQ BIT(15)
> +#define LTR_MST_NO_SNOOP_SHIFT 16
> +
> +#define APPL_LTR_MSG_2 0xC8
> +#define APPL_LTR_MSG_2_LTR_MSG_REQ_STATE BIT(3)
> +
> +#define APPL_LINK_STATUS 0xCC
> +#define APPL_LINK_STATUS_RDLH_LINK_UP BIT(0)
> +
> +#define APPL_DEBUG 0xD0
> +#define APPL_DEBUG_PM_LINKST_IN_L2_LAT BIT(21)
> +#define APPL_DEBUG_PM_LINKST_IN_L0 0x11
> +#define APPL_DEBUG_LTSSM_STATE_MASK GENMASK(8, 3)
> +#define APPL_DEBUG_LTSSM_STATE_SHIFT 3
> +#define LTSSM_STATE_PRE_DETECT 5
> +
> +#define APPL_RADM_STATUS 0xE4
> +#define APPL_PM_XMT_TURNOFF_STATE BIT(0)
> +
> +#define APPL_DM_TYPE 0x100
> +#define APPL_DM_TYPE_MASK GENMASK(3, 0)
> +#define APPL_DM_TYPE_RP 0x4
> +#define APPL_DM_TYPE_EP 0x0
> +
> +#define APPL_CFG_BASE_ADDR 0x104
> +#define APPL_CFG_BASE_ADDR_MASK GENMASK(31, 12)
> +
> +#define APPL_CFG_IATU_DMA_BASE_ADDR 0x108
> +#define APPL_CFG_IATU_DMA_BASE_ADDR_MASK GENMASK(31, 18)
> +
> +#define APPL_CFG_MISC 0x110
> +#define APPL_CFG_MISC_SLV_EP_MODE BIT(14)
> +#define APPL_CFG_MISC_ARCACHE_MASK GENMASK(13, 10)
> +#define APPL_CFG_MISC_ARCACHE_SHIFT 10
> +#define APPL_CFG_MISC_ARCACHE_VAL 3
> +
> +#define APPL_CFG_SLCG_OVERRIDE 0x114
> +#define APPL_CFG_SLCG_OVERRIDE_SLCG_EN_MASTER BIT(0)
> +
> +#define APPL_CAR_RESET_OVRD 0x12C
> +#define APPL_CAR_RESET_OVRD_CYA_OVERRIDE_CORE_RST_N BIT(0)
> +
> +#define IO_BASE_IO_DECODE BIT(0)
> +#define IO_BASE_IO_DECODE_BIT8 BIT(8)
> +
> +#define CFG_PREF_MEM_LIMIT_BASE_MEM_DECODE BIT(0)
> +#define CFG_PREF_MEM_LIMIT_BASE_MEM_LIMIT_DECODE BIT(16)
> +
> +#define CFG_TIMER_CTRL_MAX_FUNC_NUM_OFF 0x718
> +#define CFG_TIMER_CTRL_ACK_NAK_SHIFT (19)
> +
> +#define EVENT_COUNTER_ALL_CLEAR 0x3
> +#define EVENT_COUNTER_ENABLE_ALL 0x7
> +#define EVENT_COUNTER_ENABLE_SHIFT 2
> +#define EVENT_COUNTER_EVENT_SEL_MASK GENMASK(7, 0)
> +#define EVENT_COUNTER_EVENT_SEL_SHIFT 16
> +#define EVENT_COUNTER_EVENT_Tx_L0S 0x2
> +#define EVENT_COUNTER_EVENT_Rx_L0S 0x3
> +#define EVENT_COUNTER_EVENT_L1 0x5
> +#define EVENT_COUNTER_EVENT_L1_1 0x7
> +#define EVENT_COUNTER_EVENT_L1_2 0x8
> +#define EVENT_COUNTER_GROUP_SEL_SHIFT 24
> +#define EVENT_COUNTER_GROUP_5 0x5
> +
> +#define PORT_LOGIC_ACK_F_ASPM_CTRL 0x70C
> +#define ENTER_ASPM BIT(30)
> +#define L0S_ENTRANCE_LAT_SHIFT 24
> +#define L0S_ENTRANCE_LAT_MASK GENMASK(26, 24)
> +#define L1_ENTRANCE_LAT_SHIFT 27
> +#define L1_ENTRANCE_LAT_MASK GENMASK(29, 27)
> +#define N_FTS_SHIFT 8
> +#define N_FTS_MASK GENMASK(7, 0)
> +#define N_FTS_VAL 52
> +
> +#define PORT_LOGIC_GEN2_CTRL 0x80C
> +#define PORT_LOGIC_GEN2_CTRL_DIRECT_SPEED_CHANGE BIT(17)
> +#define FTS_MASK GENMASK(7, 0)
> +#define FTS_VAL 52
> +
> +#define PORT_LOGIC_MSI_CTRL_INT_0_EN 0x828
> +
> +#define GEN3_EQ_CONTROL_OFF 0x8a8
> +#define GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC_SHIFT 8
> +#define GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC_MASK GENMASK(23, 8)
> +#define GEN3_EQ_CONTROL_OFF_FB_MODE_MASK GENMASK(3, 0)
> +
> +#define GEN3_RELATED_OFF 0x890
> +#define GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL BIT(0)
> +#define GEN3_RELATED_OFF_GEN3_EQ_DISABLE BIT(16)
> +#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT 24
> +#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK GENMASK(25, 24)
> +
> +#define PORT_LOGIC_AMBA_ERROR_RESPONSE_DEFAULT 0x8D0
> +#define AMBA_ERROR_RESPONSE_CRS_SHIFT 3
> +#define AMBA_ERROR_RESPONSE_CRS_MASK GENMASK(1, 0)
> +#define AMBA_ERROR_RESPONSE_CRS_OKAY 0
> +#define AMBA_ERROR_RESPONSE_CRS_OKAY_FFFFFFFF 1
> +#define AMBA_ERROR_RESPONSE_CRS_OKAY_FFFF0001 2
> +
> +#define PORT_LOGIC_MSIX_DOORBELL 0x948
> +
> +#define CAP_SPCIE_CAP_OFF 0x154
> +#define CAP_SPCIE_CAP_OFF_DSP_TX_PRESET0_MASK GENMASK(3, 0)
> +#define CAP_SPCIE_CAP_OFF_USP_TX_PRESET0_MASK GENMASK(11, 8)
> +#define CAP_SPCIE_CAP_OFF_USP_TX_PRESET0_SHIFT 8
> +
> +#define PL16G_CAP_OFF_DSP_16G_TX_PRESET_MASK GENMASK(3, 0)
> +#define PL16G_CAP_OFF_USP_16G_TX_PRESET_MASK GENMASK(7, 4)
> +#define PL16G_CAP_OFF_USP_16G_TX_PRESET_SHIFT 4
> +
> +#define PME_ACK_TIMEOUT 10000
> +
> +#define LTSSM_TIMEOUT 50000 /* 50ms */
> +
> +#define GEN3_GEN4_EQ_PRESET_INIT 5
> +
> +#define GEN1_CORE_CLK_FREQ 62500000
> +#define GEN2_CORE_CLK_FREQ 125000000
> +#define GEN3_CORE_CLK_FREQ 250000000
> +#define GEN4_CORE_CLK_FREQ 500000000
> +
> +static const unsigned int pcie_gen_freq[] = {
> + GEN1_CORE_CLK_FREQ,
> + GEN2_CORE_CLK_FREQ,
> + GEN3_CORE_CLK_FREQ,
> + GEN4_CORE_CLK_FREQ
> +};
> +
> +static const u32 event_cntr_ctrl_offset[] = {
> + 0x1d8,
> + 0x1a8,
> + 0x1a8,
> + 0x1a8,
> + 0x1c4,
> + 0x1d8
> +};
> +
> +static const u32 event_cntr_data_offset[] = {
> + 0x1dc,
> + 0x1ac,
> + 0x1ac,
> + 0x1ac,
> + 0x1c8,
> + 0x1dc
> +};
> +
> +struct tegra_pcie_dw {
> + struct device *dev;
> + struct resource *appl_res;
> + struct resource *dbi_res;
> + struct resource *atu_dma_res;
> + void __iomem *appl_base;
> + struct clk *core_clk;
> + struct reset_control *core_apb_rst;
> + struct reset_control *core_rst;
> + struct dw_pcie pci;
> + enum dw_pcie_device_mode mode;
> + struct tegra_bpmp *bpmp;
> +
> + bool supports_clkreq;
> + u8 init_link_width;
> + bool link_state;
> + u32 msi_ctrl_int;
> + u32 num_lanes;
> + u32 max_speed;
> + u32 cid;
> + bool update_fc_fixup;
> + u32 cfg_link_cap_l1sub;
> + u32 pcie_cap_base;
> + u32 aspm_cmrt;
> + u32 aspm_pwr_on_t;
> + u32 aspm_l0s_enter_lat;
> +
> + struct regulator *pex_ctl_supply;
> +
> + u32 phy_count;

This doesn't need to be a sized type. unsigned int would do just fine.

> + struct phy **phys;
> +
> + struct dentry *debugfs;
> +};
> +
> +static inline struct tegra_pcie_dw *to_tegra_pcie(struct dw_pcie *pci)
> +{
> + return container_of(pci, struct tegra_pcie_dw, pci);
> +}
> +
> +struct tegra_pcie_soc {
> + enum dw_pcie_device_mode mode;
> +};
> +
> +static void apply_bad_link_workaround(struct pcie_port *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct tegra_pcie_dw *pcie = to_tegra_pcie(pci);
> + u32 current_link_width;
> + u16 val;
> +
> + /*
> + * NOTE:- Since this scenario is uncommon and link as such is not
> + * stable anyway, not waiting to confirm if link is really
> + * transitioning to Gen-2 speed
> + */
> + val = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA);
> + if (val & PCI_EXP_LNKSTA_LBMS) {
> + current_link_width = (val & PCI_EXP_LNKSTA_NLW) >>
> + PCI_EXP_LNKSTA_NLW_SHIFT;
> + if (pcie->init_link_width > current_link_width) {
> + dev_warn(pci->dev, "PCIe link is bad, width reduced\n");
> + val = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base +
> + PCI_EXP_LNKCTL2);
> + val &= ~PCI_EXP_LNKCTL2_TLS;
> + val |= PCI_EXP_LNKCTL2_TLS_2_5GT;
> + dw_pcie_writew_dbi(pci, pcie->pcie_cap_base +
> + PCI_EXP_LNKCTL2, val);
> +
> + val = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base +
> + PCI_EXP_LNKCTL);
> + val |= PCI_EXP_LNKCTL_RL;
> + dw_pcie_writew_dbi(pci, pcie->pcie_cap_base +
> + PCI_EXP_LNKCTL, val);
> + }
> + }
> +}
> +
> +static irqreturn_t tegra_pcie_rp_irq_handler(struct tegra_pcie_dw *pcie)
> +{
> + struct dw_pcie *pci = &pcie->pci;
> + struct pcie_port *pp = &pci->pp;
> + u32 val, tmp;
> + u16 val_w;
> +
> + val = readl(pcie->appl_base + APPL_INTR_STATUS_L0);
> + if (val & APPL_INTR_STATUS_L0_LINK_STATE_INT) {
> + val = readl(pcie->appl_base + APPL_INTR_STATUS_L1_0_0);
> + if (val & APPL_INTR_STATUS_L1_0_0_LINK_REQ_RST_NOT_CHGED) {
> + writel(val, pcie->appl_base + APPL_INTR_STATUS_L1_0_0);
> +
> + /* SBR & Surprise Link Down WAR */
> + val = readl(pcie->appl_base + APPL_CAR_RESET_OVRD);
> + val &= ~APPL_CAR_RESET_OVRD_CYA_OVERRIDE_CORE_RST_N;
> + writel(val, pcie->appl_base + APPL_CAR_RESET_OVRD);
> + udelay(1);
> + val = readl(pcie->appl_base + APPL_CAR_RESET_OVRD);
> + val |= APPL_CAR_RESET_OVRD_CYA_OVERRIDE_CORE_RST_N;
> + writel(val, pcie->appl_base + APPL_CAR_RESET_OVRD);
> +
> + val = dw_pcie_readl_dbi(pci, PORT_LOGIC_GEN2_CTRL);
> + val |= PORT_LOGIC_GEN2_CTRL_DIRECT_SPEED_CHANGE;
> + dw_pcie_writel_dbi(pci, PORT_LOGIC_GEN2_CTRL, val);
> + }
> + }
> + if (val & APPL_INTR_STATUS_L0_INT_INT) {

I think you could use a blank line here (and elsewhere) to separate
blocks of code for better readability.

> + val = readl(pcie->appl_base + APPL_INTR_STATUS_L1_8_0);
> + if (val & APPL_INTR_STATUS_L1_8_0_AUTO_BW_INT_STS) {
> + writel(APPL_INTR_STATUS_L1_8_0_AUTO_BW_INT_STS,
> + pcie->appl_base + APPL_INTR_STATUS_L1_8_0);
> + apply_bad_link_workaround(pp);
> + }
> + if (val & APPL_INTR_STATUS_L1_8_0_BW_MGT_INT_STS) {
> + writel(APPL_INTR_STATUS_L1_8_0_BW_MGT_INT_STS,
> + pcie->appl_base + APPL_INTR_STATUS_L1_8_0);
> +
> + val_w = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base +
> + PCI_EXP_LNKSTA);
> + dev_dbg(pci->dev, "Link Speed : Gen-%u\n", val_w &
> + PCI_EXP_LNKSTA_CLS);
> + }
> + }
> + val = readl(pcie->appl_base + APPL_INTR_STATUS_L0);
> + if (val & APPL_INTR_STATUS_L0_CDM_REG_CHK_INT) {
> + val = readl(pcie->appl_base + APPL_INTR_STATUS_L1_18);
> + tmp = dw_pcie_readl_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS);
> + if (val & APPL_INTR_STATUS_L1_18_CDM_REG_CHK_CMPLT) {
> + dev_info(pci->dev, "CDM check complete\n");
> + tmp |= PCIE_PL_CHK_REG_CHK_REG_COMPLETE;
> + }
> + if (val & APPL_INTR_STATUS_L1_18_CDM_REG_CHK_CMP_ERR) {
> + dev_err(pci->dev, "CDM comparison mismatch\n");
> + tmp |= PCIE_PL_CHK_REG_CHK_REG_COMPARISON_ERROR;
> + }
> + if (val & APPL_INTR_STATUS_L1_18_CDM_REG_CHK_LOGIC_ERR) {
> + dev_err(pci->dev, "CDM Logic error\n");
> + tmp |= PCIE_PL_CHK_REG_CHK_REG_LOGIC_ERROR;
> + }
> + dw_pcie_writel_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS, tmp);
> + tmp = dw_pcie_readl_dbi(pci, PCIE_PL_CHK_REG_ERR_ADDR);
> + dev_err(pci->dev, "CDM Error Address Offset = 0x%08X\n", tmp);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t tegra_pcie_irq_handler(int irq, void *arg)
> +{
> + struct tegra_pcie_dw *pcie = (struct tegra_pcie_dw *)arg;

No need for this explicit cast.

> +
> + if (pcie->mode == DW_PCIE_RC_TYPE)
> + return tegra_pcie_rp_irq_handler(pcie);
> +
> + return IRQ_NONE;
> +}
> +
> +static int tegra_pcie_dw_rd_own_conf(struct pcie_port *pp, int where, int size,
> + u32 *val)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
> + /*
> + * This is an endpoint mode specific register happen to appear even
> + * when controller is operating in root port mode and system hangs
> + * when it is accessed with link being in ASPM-L1 state.
> + * So skip accessing it altogether
> + */
> + if (where == PORT_LOGIC_MSIX_DOORBELL) {
> + *val = 0x00000000;
> + return PCIBIOS_SUCCESSFUL;
> + }
> +
> + return dw_pcie_read(pci->dbi_base + where, size, val);
> +}
> +
> +static int tegra_pcie_dw_wr_own_conf(struct pcie_port *pp, int where, int size,
> + u32 val)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
> + /*
> + * This is an endpoint mode specific register happen to appear even
> + * when controller is operating in root port mode and system hangs
> + * when it is accessed with link being in ASPM-L1 state.
> + * So skip accessing it altogether
> + */
> + if (where == PORT_LOGIC_MSIX_DOORBELL)
> + return PCIBIOS_SUCCESSFUL;
> +
> + return dw_pcie_write(pci->dbi_base + where, size, val);
> +}
> +
> +#if defined(CONFIG_PCIEASPM)
> +static void disable_aspm_l11(struct tegra_pcie_dw *pcie)
> +{
> + u32 val;
> +
> + val = dw_pcie_readl_dbi(&pcie->pci, pcie->cfg_link_cap_l1sub);
> + val &= ~PCI_L1SS_CAP_ASPM_L1_1;
> + dw_pcie_writel_dbi(&pcie->pci, pcie->cfg_link_cap_l1sub, val);
> +}
> +
> +static void disable_aspm_l12(struct tegra_pcie_dw *pcie)
> +{
> + u32 val;
> +
> + val = dw_pcie_readl_dbi(&pcie->pci, pcie->cfg_link_cap_l1sub);
> + val &= ~PCI_L1SS_CAP_ASPM_L1_2;
> + dw_pcie_writel_dbi(&pcie->pci, pcie->cfg_link_cap_l1sub, val);
> +}
> +
> +static inline u32 event_counter_prog(struct tegra_pcie_dw *pcie, u32 event)
> +{
> + u32 val;
> +
> + val = dw_pcie_readl_dbi(&pcie->pci, event_cntr_ctrl_offset[pcie->cid]);
> + val &= ~(EVENT_COUNTER_EVENT_SEL_MASK << EVENT_COUNTER_EVENT_SEL_SHIFT);
> + val |= EVENT_COUNTER_GROUP_5 << EVENT_COUNTER_GROUP_SEL_SHIFT;
> + val |= event << EVENT_COUNTER_EVENT_SEL_SHIFT;
> + val |= EVENT_COUNTER_ENABLE_ALL << EVENT_COUNTER_ENABLE_SHIFT;
> + dw_pcie_writel_dbi(&pcie->pci, event_cntr_ctrl_offset[pcie->cid], val);
> + val = dw_pcie_readl_dbi(&pcie->pci, event_cntr_data_offset[pcie->cid]);
> + return val;
> +}
> +
> +static int aspm_state_cnt(struct seq_file *s, void *data)
> +{
> + struct tegra_pcie_dw *pcie = (struct tegra_pcie_dw *)
> + dev_get_drvdata(s->private);
> + u32 val;
> +
> + seq_printf(s, "Tx L0s entry count : %u\n",
> + event_counter_prog(pcie, EVENT_COUNTER_EVENT_Tx_L0S));
> +
> + seq_printf(s, "Rx L0s entry count : %u\n",
> + event_counter_prog(pcie, EVENT_COUNTER_EVENT_Rx_L0S));
> +
> + seq_printf(s, "Link L1 entry count : %u\n",
> + event_counter_prog(pcie, EVENT_COUNTER_EVENT_L1));
> +
> + seq_printf(s, "Link L1.1 entry count : %u\n",
> + event_counter_prog(pcie, EVENT_COUNTER_EVENT_L1_1));
> +
> + seq_printf(s, "Link L1.2 entry count : %u\n",
> + event_counter_prog(pcie, EVENT_COUNTER_EVENT_L1_2));
> +
> + /* Clear all counters */
> + dw_pcie_writel_dbi(&pcie->pci, event_cntr_ctrl_offset[pcie->cid],
> + EVENT_COUNTER_ALL_CLEAR);
> +
> + /* Re-enable counting */
> + val = EVENT_COUNTER_ENABLE_ALL << EVENT_COUNTER_ENABLE_SHIFT;
> + val |= EVENT_COUNTER_GROUP_5 << EVENT_COUNTER_GROUP_SEL_SHIFT;
> + dw_pcie_writel_dbi(&pcie->pci, event_cntr_ctrl_offset[pcie->cid], val);
> +
> + return 0;
> +}
> +#endif
> +
> +static int init_debugfs(struct tegra_pcie_dw *pcie)
> +{
> +#if defined(CONFIG_PCIEASPM)
> + struct dentry *d;
> +
> + d = debugfs_create_devm_seqfile(pcie->dev, "aspm_state_cnt",
> + pcie->debugfs, aspm_state_cnt);
> + if (IS_ERR_OR_NULL(d))
> + dev_err(pcie->dev,
> + "Failed to create debugfs file \"aspm_state_cnt\"\n");
> +#endif
> + return 0;
> +}
> +
> +static void tegra_pcie_enable_system_interrupts(struct pcie_port *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct tegra_pcie_dw *pcie = to_tegra_pcie(pci);
> + u32 val;
> + u16 val_w;
> +
> + val = readl(pcie->appl_base + APPL_INTR_EN_L0_0);
> + val |= APPL_INTR_EN_L0_0_LINK_STATE_INT_EN;
> + writel(val, pcie->appl_base + APPL_INTR_EN_L0_0);
> +
> + val = readl(pcie->appl_base + APPL_INTR_EN_L1_0_0);
> + val |= APPL_INTR_EN_L1_0_0_LINK_REQ_RST_NOT_INT_EN;
> + writel(val, pcie->appl_base + APPL_INTR_EN_L1_0_0);
> +
> + if (of_property_read_bool(pci->dev->of_node, "cdm-check")) {

This doesn't match the DT bindings if I remember correctly. It might
also be worth reading this at *_dt_parse() time and storing the result
in a field of struct tegra_pcie_dw.

> + val = readl(pcie->appl_base + APPL_INTR_EN_L0_0);
> + val |= APPL_INTR_EN_L0_0_CDM_REG_CHK_INT_EN;
> + writel(val, pcie->appl_base + APPL_INTR_EN_L0_0);
> +
> + val = readl(pcie->appl_base + APPL_INTR_EN_L1_18);
> + val |= APPL_INTR_EN_L1_18_CDM_REG_CHK_CMP_ERR;
> + val |= APPL_INTR_EN_L1_18_CDM_REG_CHK_LOGIC_ERR;
> + writel(val, pcie->appl_base + APPL_INTR_EN_L1_18);
> + }
> +
> + val_w = dw_pcie_readw_dbi(&pcie->pci, pcie->pcie_cap_base +
> + PCI_EXP_LNKSTA);
> + pcie->init_link_width = (val_w & PCI_EXP_LNKSTA_NLW) >>
> + PCI_EXP_LNKSTA_NLW_SHIFT;
> +
> + val_w = dw_pcie_readw_dbi(&pcie->pci, pcie->pcie_cap_base +
> + PCI_EXP_LNKCTL);
> + val_w |= PCI_EXP_LNKCTL_LBMIE;
> + dw_pcie_writew_dbi(&pcie->pci, pcie->pcie_cap_base + PCI_EXP_LNKCTL,
> + val_w);
> +}
> +
> +static void tegra_pcie_enable_legacy_interrupts(struct pcie_port *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct tegra_pcie_dw *pcie = to_tegra_pcie(pci);
> + u32 val;
> +
> + /* Enable legacy interrupt generation */
> + val = readl(pcie->appl_base + APPL_INTR_EN_L0_0);
> + val |= APPL_INTR_EN_L0_0_SYS_INTR_EN;
> + val |= APPL_INTR_EN_L0_0_INT_INT_EN;
> + writel(val, pcie->appl_base + APPL_INTR_EN_L0_0);
> +
> + val = readl(pcie->appl_base + APPL_INTR_EN_L1_8_0);
> + val |= APPL_INTR_EN_L1_8_INTX_EN;
> + val |= APPL_INTR_EN_L1_8_AUTO_BW_INT_EN;
> + val |= APPL_INTR_EN_L1_8_BW_MGT_INT_EN;
> + if (IS_ENABLED(CONFIG_PCIEAER))
> + val |= APPL_INTR_EN_L1_8_AER_INT_EN;
> + writel(val, pcie->appl_base + APPL_INTR_EN_L1_8_0);
> +}
> +
> +static void tegra_pcie_enable_msi_interrupts(struct pcie_port *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct tegra_pcie_dw *pcie = to_tegra_pcie(pci);
> + u32 val;
> +
> + dw_pcie_msi_init(pp);
> +
> + /* Enable MSI interrupt generation */
> + val = readl(pcie->appl_base + APPL_INTR_EN_L0_0);
> + val |= APPL_INTR_EN_L0_0_SYS_MSI_INTR_EN;
> + val |= APPL_INTR_EN_L0_0_MSI_RCV_INT_EN;
> + writel(val, pcie->appl_base + APPL_INTR_EN_L0_0);
> +}
> +
> +static void tegra_pcie_enable_interrupts(struct pcie_port *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct tegra_pcie_dw *pcie = to_tegra_pcie(pci);
> +
> + /* Clear interrupt statuses before enabling interrupts */
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L0);
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L1_0_0);
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L1_1);
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L1_2);
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L1_3);
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L1_6);
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L1_7);
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L1_8_0);
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L1_9);
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L1_10);
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L1_11);
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L1_13);
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L1_14);
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L1_15);
> + writel(0xFFFFFFFF, pcie->appl_base + APPL_INTR_STATUS_L1_17);
> +
> + tegra_pcie_enable_system_interrupts(pp);
> + tegra_pcie_enable_legacy_interrupts(pp);
> + if (IS_ENABLED(CONFIG_PCI_MSI))
> + tegra_pcie_enable_msi_interrupts(pp);
> +}
> +
> +static void config_gen3_gen4_eq_presets(struct tegra_pcie_dw *pcie)
> +{
> + struct dw_pcie *pci = &pcie->pci;
> + u32 val, offset, i;
> +
> + /* Program init preset */
> + for (i = 0; i < pcie->num_lanes; i++) {
> + dw_pcie_read(pci->dbi_base + CAP_SPCIE_CAP_OFF
> + + (i * 2), 2, &val);
> + val &= ~CAP_SPCIE_CAP_OFF_DSP_TX_PRESET0_MASK;
> + val |= GEN3_GEN4_EQ_PRESET_INIT;
> + val &= ~CAP_SPCIE_CAP_OFF_USP_TX_PRESET0_MASK;
> + val |= (GEN3_GEN4_EQ_PRESET_INIT <<
> + CAP_SPCIE_CAP_OFF_USP_TX_PRESET0_SHIFT);
> + dw_pcie_write(pci->dbi_base + CAP_SPCIE_CAP_OFF
> + + (i * 2), 2, val);
> +
> + offset = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_PL) +
> + PCI_PL_16GT_LE_CTRL;
> + dw_pcie_read(pci->dbi_base + offset + i, 1, &val);
> + val &= ~PL16G_CAP_OFF_DSP_16G_TX_PRESET_MASK;
> + val |= GEN3_GEN4_EQ_PRESET_INIT;
> + val &= ~PL16G_CAP_OFF_USP_16G_TX_PRESET_MASK;
> + val |= (GEN3_GEN4_EQ_PRESET_INIT <<
> + PL16G_CAP_OFF_USP_16G_TX_PRESET_SHIFT);
> + dw_pcie_write(pci->dbi_base + offset + i, 1, val);
> + }
> +
> + val = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF);
> + val &= ~GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK;
> + dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
> +
> + val = dw_pcie_readl_dbi(pci, GEN3_EQ_CONTROL_OFF);
> + val &= ~GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC_MASK;
> + val |= (0x3ff << GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC_SHIFT);
> + val &= ~GEN3_EQ_CONTROL_OFF_FB_MODE_MASK;
> + dw_pcie_writel_dbi(pci, GEN3_EQ_CONTROL_OFF, val);
> +
> + val = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF);
> + val &= ~GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK;
> + val |= (0x1 << GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT);
> + dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
> +
> + val = dw_pcie_readl_dbi(pci, GEN3_EQ_CONTROL_OFF);
> + val &= ~GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC_MASK;
> + val |= (0x360 << GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC_SHIFT);
> + val &= ~GEN3_EQ_CONTROL_OFF_FB_MODE_MASK;
> + dw_pcie_writel_dbi(pci, GEN3_EQ_CONTROL_OFF, val);
> +
> + val = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF);
> + val &= ~GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK;
> + dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
> +}
> +
> +static int tegra_pcie_dw_host_init(struct pcie_port *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct tegra_pcie_dw *pcie = to_tegra_pcie(pci);
> + u32 val, tmp, offset, speed;
> + int count;

Can be unsigned int.

> + u16 val_w;
> +
> +core_init:
> + count = 200;
> +#if defined(CONFIG_PCIEASPM)
> + offset = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_L1SS);
> + pcie->cfg_link_cap_l1sub = offset + PCI_L1SS_CAP;
> +#endif
> + val = dw_pcie_readl_dbi(pci, PCI_IO_BASE);
> + val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8);
> + dw_pcie_writel_dbi(pci, PCI_IO_BASE, val);
> +
> + val = dw_pcie_readl_dbi(pci, PCI_PREF_MEMORY_BASE);
> + val |= CFG_PREF_MEM_LIMIT_BASE_MEM_DECODE;
> + val |= CFG_PREF_MEM_LIMIT_BASE_MEM_LIMIT_DECODE;
> + dw_pcie_writel_dbi(pci, PCI_PREF_MEMORY_BASE, val);
> +
> + dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 0);
> +
> + /* Configure FTS */
> + val = dw_pcie_readl_dbi(pci, PORT_LOGIC_ACK_F_ASPM_CTRL);
> + val &= ~(N_FTS_MASK << N_FTS_SHIFT);
> + val |= N_FTS_VAL << N_FTS_SHIFT;
> + dw_pcie_writel_dbi(pci, PORT_LOGIC_ACK_F_ASPM_CTRL, val);
> +
> + val = dw_pcie_readl_dbi(pci, PORT_LOGIC_GEN2_CTRL);
> + val &= ~FTS_MASK;
> + val |= FTS_VAL;
> + dw_pcie_writel_dbi(pci, PORT_LOGIC_GEN2_CTRL, val);
> +
> + /* Enable as 0xFFFF0001 response for CRS */
> + val = dw_pcie_readl_dbi(pci, PORT_LOGIC_AMBA_ERROR_RESPONSE_DEFAULT);
> + val &= ~(AMBA_ERROR_RESPONSE_CRS_MASK << AMBA_ERROR_RESPONSE_CRS_SHIFT);
> + val |= (AMBA_ERROR_RESPONSE_CRS_OKAY_FFFF0001 <<
> + AMBA_ERROR_RESPONSE_CRS_SHIFT);
> + dw_pcie_writel_dbi(pci, PORT_LOGIC_AMBA_ERROR_RESPONSE_DEFAULT, val);
> +
> + /* Configure Max Speed from DT */
> + if (pcie->max_speed && pcie->max_speed != -EINVAL) {
> + val = dw_pcie_readl_dbi(pci, pcie->pcie_cap_base +
> + PCI_EXP_LNKCAP);
> + val &= ~PCI_EXP_LNKCAP_SLS;
> + val |= pcie->max_speed;
> + dw_pcie_writel_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKCAP,
> + val);
> + }
> +
> + /* Configure Max lane width from DT */
> + val = dw_pcie_readl_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKCAP);
> + val &= ~PCI_EXP_LNKCAP_MLW;
> + val |= (pcie->num_lanes << PCI_EXP_LNKSTA_NLW_SHIFT);
> + dw_pcie_writel_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKCAP, val);
> +
> + config_gen3_gen4_eq_presets(pcie);
> +
> +#if defined(CONFIG_PCIEASPM)
> + /* Enable ASPM counters */
> + val = EVENT_COUNTER_ENABLE_ALL << EVENT_COUNTER_ENABLE_SHIFT;
> + val |= EVENT_COUNTER_GROUP_5 << EVENT_COUNTER_GROUP_SEL_SHIFT;
> + dw_pcie_writel_dbi(pci, event_cntr_ctrl_offset[pcie->cid], val);
> +
> + /* Program T_cmrt and T_pwr_on values */
> + val = dw_pcie_readl_dbi(pci, pcie->cfg_link_cap_l1sub);
> + val &= ~(PCI_L1SS_CAP_CM_RESTORE_TIME | PCI_L1SS_CAP_P_PWR_ON_VALUE);
> + val |= (pcie->aspm_cmrt << 8);
> + val |= (pcie->aspm_pwr_on_t << 19);
> + dw_pcie_writel_dbi(pci, pcie->cfg_link_cap_l1sub, val);
> +
> + /* Program L0s and L1 entrance latencies */
> + val = dw_pcie_readl_dbi(pci, PORT_LOGIC_ACK_F_ASPM_CTRL);
> + val &= ~L0S_ENTRANCE_LAT_MASK;
> + val |= (pcie->aspm_l0s_enter_lat << L0S_ENTRANCE_LAT_SHIFT);
> + val |= ENTER_ASPM;
> + dw_pcie_writel_dbi(pci, PORT_LOGIC_ACK_F_ASPM_CTRL, val);
> +#endif
> + val = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF);
> + val &= ~GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL;
> + dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
> +
> + if (pcie->update_fc_fixup) {
> + val = dw_pcie_readl_dbi(pci, CFG_TIMER_CTRL_MAX_FUNC_NUM_OFF);
> + val |= 0x1 << CFG_TIMER_CTRL_ACK_NAK_SHIFT;
> + dw_pcie_writel_dbi(pci, CFG_TIMER_CTRL_MAX_FUNC_NUM_OFF, val);
> + }
> +
> + dw_pcie_setup_rc(pp);
> +
> + clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ);
> +
> + /* Assert RST */
> + val = readl(pcie->appl_base + APPL_PINMUX);
> + val &= ~APPL_PINMUX_PEX_RST;
> + writel(val, pcie->appl_base + APPL_PINMUX);
> +
> + usleep_range(100, 200);
> +
> + /* Enable LTSSM */
> + val = readl(pcie->appl_base + APPL_CTRL);
> + val |= APPL_CTRL_LTSSM_EN;
> + writel(val, pcie->appl_base + APPL_CTRL);
> +
> + /* De-assert RST */
> + val = readl(pcie->appl_base + APPL_PINMUX);
> + val |= APPL_PINMUX_PEX_RST;
> + writel(val, pcie->appl_base + APPL_PINMUX);
> +
> + msleep(100);
> +
> + val_w = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA);
> + while (!(val_w & PCI_EXP_LNKSTA_DLLLA)) {
> + if (count) {
> + dev_dbg(pci->dev, "Waiting for link up\n");
> + usleep_range(1000, 2000);
> + val_w = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base +
> + PCI_EXP_LNKSTA);
> + count--;
> + continue;
> + }
> +
> + val = readl(pcie->appl_base + APPL_DEBUG);
> + val &= APPL_DEBUG_LTSSM_STATE_MASK;
> + val >>= APPL_DEBUG_LTSSM_STATE_SHIFT;
> + tmp = readl(pcie->appl_base + APPL_LINK_STATUS);
> + tmp &= APPL_LINK_STATUS_RDLH_LINK_UP;
> + if (!(val == 0x11 && !tmp)) {
> + dev_info(pci->dev, "Link is down\n");
> + return 0;
> + }
> +
> + dev_info(pci->dev, "Link is down in DLL");
> + dev_info(pci->dev, "Trying again with DLFE disabled\n");
> + /* Disable LTSSM */
> + val = readl(pcie->appl_base + APPL_CTRL);
> + val &= ~APPL_CTRL_LTSSM_EN;
> + writel(val, pcie->appl_base + APPL_CTRL);
> +
> + reset_control_assert(pcie->core_rst);
> + reset_control_deassert(pcie->core_rst);
> +
> + offset = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_DLF);
> + val = dw_pcie_readl_dbi(pci, offset + PCI_DLF_CAP);
> + val &= ~PCI_DLF_EXCHANGE_ENABLE;
> + dw_pcie_writel_dbi(pci, offset, val);
> +
> + /* Retry now with DLF Exchange disabled */
> + goto core_init;
> + }
> + dev_dbg(pci->dev, "Link is up\n");
> +
> + speed = (dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA) &
> + PCI_EXP_LNKSTA_CLS);

No need for the outer parentheses.

> + clk_set_rate(pcie->core_clk, pcie_gen_freq[speed - 1]);
> +
> + tegra_pcie_enable_interrupts(pp);
> +
> + return 0;
> +}
> +
> +static int tegra_pcie_dw_link_up(struct dw_pcie *pci)
> +{
> + struct tegra_pcie_dw *pcie = to_tegra_pcie(pci);
> + u32 val = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA);
> +
> + return !!(val & PCI_EXP_LNKSTA_DLLLA);
> +}
> +
> +static void tegra_pcie_set_msi_vec_num(struct pcie_port *pp)
> +{
> + pp->num_vectors = MAX_MSI_IRQS;
> +}
> +
> +static const struct dw_pcie_ops tegra_dw_pcie_ops = {
> + .link_up = tegra_pcie_dw_link_up,
> +};
> +
> +static struct dw_pcie_host_ops tegra_pcie_dw_host_ops = {
> + .rd_own_conf = tegra_pcie_dw_rd_own_conf,
> + .wr_own_conf = tegra_pcie_dw_wr_own_conf,
> + .host_init = tegra_pcie_dw_host_init,
> + .set_num_vectors = tegra_pcie_set_msi_vec_num,
> +};
> +
> +static void tegra_pcie_disable_phy(struct tegra_pcie_dw *pcie)
> +{
> + u32 phy_count = pcie->phy_count;
> +
> + while (phy_count--) {
> + phy_power_off(pcie->phys[phy_count]);
> + phy_exit(pcie->phys[phy_count]);
> + }
> +}
> +
> +static int tegra_pcie_enable_phy(struct tegra_pcie_dw *pcie)
> +{
> + u32 phy_count = pcie->phy_count;

This seems unnecessary in this function.

> + int ret;
> + u32 i;

Can be unsigned int to match pcie->phy_count.

> +
> + for (i = 0; i < phy_count; i++) {
> + ret = phy_init(pcie->phys[i]);
> + if (ret < 0)
> + goto err_phy_init;
> +
> + ret = phy_power_on(pcie->phys[i]);
> + if (ret < 0) {
> + phy_exit(pcie->phys[i]);
> + goto err_phy_power_on;
> + }
> + }
> +
> + return 0;
> +
> + while (i >= 0) {

Oh... looks like this can't be unsigned, otherwise you'll be infinitely
repeating this loop. Perhaps you can rewrite it like this to avoid that:

while (i--) {
...
}

That's the most idiomatic way to unwind in my opinion.

> + phy_power_off(pcie->phys[i]);
> +err_phy_power_on:
> + phy_exit(pcie->phys[i]);
> +err_phy_init:
> + i--;
> + }
> +
> + return ret;
> +}
> +
> +static int tegra_pcie_dw_parse_dt(struct tegra_pcie_dw *pcie)
> +{
> + struct device_node *np = pcie->dev->of_node;
> + int ret;
> +
> + ret = of_property_read_u32(np, "nvidia,aspm-cmrt-us", &pcie->aspm_cmrt);
> + if (ret < 0) {
> + dev_info(pcie->dev, "Failed to read ASPM T_cmrt: %d\n", ret);
> + return ret;
> + }
> +
> + ret = of_property_read_u32(np, "nvidia,aspm-pwr-on-t-us",
> + &pcie->aspm_pwr_on_t);
> + if (ret < 0)
> + dev_info(pcie->dev, "Failed to read ASPM Power On time: %d\n",
> + ret);
> +
> + ret = of_property_read_u32(np, "nvidia,aspm-l0s-entrance-latency-us",
> + &pcie->aspm_l0s_enter_lat);
> + if (ret < 0)
> + dev_info(pcie->dev,
> + "Failed to read ASPM L0s Entrance latency: %d\n", ret);
> +
> + ret = of_property_read_u32(np, "num-lanes", &pcie->num_lanes);
> + if (ret < 0) {
> + dev_err(pcie->dev, "Failed to read num-lanes: %d\n", ret);
> + return ret;
> + }
> +
> + pcie->max_speed = of_pci_get_max_link_speed(np);
> +
> + ret = of_property_read_u32_index(np, "nvidia,bpmp", 1, &pcie->cid);
> + if (ret) {
> + dev_err(pcie->dev, "Failed to read Controller-ID: %d\n", ret);
> + return ret;
> + }
> +
> + pcie->phy_count = of_property_count_strings(np, "phy-names");
> + if (pcie->phy_count < 0) {
> + dev_err(pcie->dev, "Failed to find phy entries\n");
> + return pcie->phy_count;
> + }
> +
> + if (of_property_read_bool(np, "nvidia,update-fc-fixup"))
> + pcie->update_fc_fixup = true;
> +
> + pcie->supports_clkreq =
> + of_property_read_bool(pcie->dev->of_node, "supports-clkreq");
> +
> + return 0;
> +}
> +
> +static int tegra_pcie_bpmp_set_ctrl_state(struct tegra_pcie_dw *pcie,
> + int enable)

Perhaps make this a boolean? That's not an exact match for the
req.controller_state.enable field defined in the ABI, but it's a bit
closer than an int. Also, boolean is effectively what the ABI allows,
so might as well use that.

> +{
> + struct mrq_uphy_response resp;
> + struct tegra_bpmp_message msg;
> + struct mrq_uphy_request req;
> + int err;
> +
> + if (pcie->cid == CTRL_5)
> + return 0;
> +
> + memset(&req, 0, sizeof(req));
> + memset(&resp, 0, sizeof(resp));
> +
> + req.cmd = CMD_UPHY_PCIE_CONTROLLER_STATE;
> + req.controller_state.pcie_controller = pcie->cid;
> + req.controller_state.enable = enable;
> +
> + memset(&msg, 0, sizeof(msg));
> + msg.mrq = MRQ_UPHY;
> + msg.tx.data = &req;
> + msg.tx.size = sizeof(req);
> + msg.rx.data = &resp;
> + msg.rx.size = sizeof(resp);
> +
> + if (irqs_disabled())
> + err = tegra_bpmp_transfer_atomic(pcie->bpmp, &msg);
> + else
> + err = tegra_bpmp_transfer(pcie->bpmp, &msg);
> +
> + return err;
> +}
> +
> +static void tegra_pcie_downstream_dev_to_D0(struct tegra_pcie_dw *pcie)
> +{
> + struct pcie_port *pp = &pcie->pci.pp;
> + struct pci_dev *pdev;
> + struct pci_bus *child;
> +
> + list_for_each_entry(child, &pp->root_bus->children, node) {
> + /* Bring downstream devices to D0 if they are not already in */
> + if (child->parent == pp->root_bus)
> + break;
> + }
> + list_for_each_entry(pdev, &child->devices, bus_list) {
> + if (PCI_FUNC(pdev->devfn) == 0) {

Why restrict this to function 0? What about other potential functions?

Also, if child devices are not in D0 at this point, isn't that an error?
Perhaps we should check the power state of the device here and output a
warning if it's not in D0 so that we can track down which device drivers
are buggy?

> + if (pci_set_power_state(pdev, PCI_D0))
> + dev_err(pcie->dev, "Transition to D0 failed\n");

Perhaps output the dev_name() of the PCI device that we failed to
transition to D0? That could help with tracking down errors.

> + }
> + }
> +}

> +
> +static int tegra_pcie_config_controller(struct tegra_pcie_dw *pcie,
> + bool en_hw_hot_rst)
> +{
> + int ret;
> + u32 val;
> +
> + ret = tegra_pcie_bpmp_set_ctrl_state(pcie, true);
> + if (ret) {
> + dev_err(pcie->dev,
> + "Failed to enable Controller-%d: %d\n",

This is an unusual way to write this. On one hand there doesn't seem to
be any consistency between error messages (see the one for failed calls
to pci_set_power_state()) and on the other hand, that Controller-%d
looks strange. Perhaps something like:

"failed to enable controller %u: %d\n", pcie->cid, ret

would be better. Note also the %u, which is preferred when printing
unsigned integers.

> + pcie->cid, ret);
> + return ret;
> + }
> +
> + ret = regulator_enable(pcie->pex_ctl_supply);
> + if (ret < 0) {
> + dev_err(pcie->dev, "Failed to enable regulator: %d\n", ret);
> + goto fail_reg_en;
> + }
> +
> + ret = clk_prepare_enable(pcie->core_clk);
> + if (ret) {
> + dev_err(pcie->dev, "Failed to enable core clock\n");

Perhaps include the error code here in order to help diagnose problems.
It looks like you have some consistency in error messages here, so feel
free to follow that in the ones that aren't consistent.

> + goto fail_core_clk;
> + }
> +
> + reset_control_deassert(pcie->core_apb_rst);
> +
> + if (en_hw_hot_rst) {
> + /* Enable HW_HOT_RST mode */
> + val = readl(pcie->appl_base + APPL_CTRL);
> + val &= ~(APPL_CTRL_HW_HOT_RST_MODE_MASK <<
> + APPL_CTRL_HW_HOT_RST_MODE_SHIFT);
> + val |= APPL_CTRL_HW_HOT_RST_EN;
> + writel(val, pcie->appl_base + APPL_CTRL);
> + }
> +
> + ret = tegra_pcie_enable_phy(pcie);
> + if (ret) {
> + dev_err(pcie->dev, "Failed to enable phy\n");

s/phy/PHY/, and perhaps include the error code.

> + goto fail_phy;
> + }
> +
> + /* Update CFG base address */
> + writel(pcie->dbi_res->start & APPL_CFG_BASE_ADDR_MASK,
> + pcie->appl_base + APPL_CFG_BASE_ADDR);
> +
> + /* Configure this core for RP mode operation */
> + writel(APPL_DM_TYPE_RP, pcie->appl_base + APPL_DM_TYPE);
> +
> + writel(0x0, pcie->appl_base + APPL_CFG_SLCG_OVERRIDE);
> +
> + val = readl(pcie->appl_base + APPL_CTRL);
> + writel(val | APPL_CTRL_SYS_PRE_DET_STATE, pcie->appl_base + APPL_CTRL);
> +
> + val = readl(pcie->appl_base + APPL_CFG_MISC);
> + val |= (APPL_CFG_MISC_ARCACHE_VAL << APPL_CFG_MISC_ARCACHE_SHIFT);
> + writel(val, pcie->appl_base + APPL_CFG_MISC);
> +
> + if (!pcie->supports_clkreq) {
> + val = readl(pcie->appl_base + APPL_PINMUX);
> + val |= APPL_PINMUX_CLKREQ_OUT_OVRD_EN;
> + val |= APPL_PINMUX_CLKREQ_OUT_OVRD;
> + writel(val, pcie->appl_base + APPL_PINMUX);
> + }
> +
> + /* Update iATU_DMA base address */
> + writel(pcie->atu_dma_res->start & APPL_CFG_IATU_DMA_BASE_ADDR_MASK,
> + pcie->appl_base + APPL_CFG_IATU_DMA_BASE_ADDR);
> +
> + reset_control_deassert(pcie->core_rst);

Technically this could fail.

> +
> + pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci,
> + PCI_CAP_ID_EXP);
> +
> +#if defined(CONFIG_PCIEASPM)
> + /* Disable ASPM-L1SS advertisement as there is no CLKREQ routing */
> + if (!pcie->supports_clkreq) {
> + disable_aspm_l11(pcie);
> + disable_aspm_l12(pcie);
> + }
> +#endif
> + return ret;
> +
> +fail_phy:
> + reset_control_assert(pcie->core_apb_rst);
> + clk_disable_unprepare(pcie->core_clk);
> +fail_core_clk:
> + regulator_disable(pcie->pex_ctl_supply);
> +fail_reg_en:
> + tegra_pcie_bpmp_set_ctrl_state(pcie, false);
> +
> + return ret;
> +}
> +
> +static int __deinit_controller(struct tegra_pcie_dw *pcie)
> +{
> + int ret;
> +
> + ret = reset_control_assert(pcie->core_rst);
> + if (ret) {
> + dev_err(pcie->dev, "Failed to assert \"core\" reset: %d\n",
> + ret);
> + return ret;
> + }
> + tegra_pcie_disable_phy(pcie);
> + ret = reset_control_assert(pcie->core_apb_rst);
> + if (ret) {
> + dev_err(pcie->dev, "Failed to assert \"core_apb\" reset: %d\n",
> + ret);
> + return ret;
> + }
> + clk_disable_unprepare(pcie->core_clk);
> + ret = regulator_disable(pcie->pex_ctl_supply);
> + if (ret) {
> + dev_err(pcie->dev, "Failed to disable regulator: %d\n", ret);
> + return ret;
> + }
> + ret = tegra_pcie_bpmp_set_ctrl_state(pcie, false);
> + if (ret) {
> + dev_err(pcie->dev, "Failed to disable controller-%d: %d\n",
> + pcie->cid, ret);
> + return ret;
> + }
> + return ret;
> +}
> +
> +static int tegra_pcie_init_controller(struct tegra_pcie_dw *pcie)
> +{
> + struct dw_pcie *pci = &pcie->pci;
> + struct pcie_port *pp = &pci->pp;
> + int ret = 0;

No need to initialize this.

> +
> + ret = tegra_pcie_config_controller(pcie, false);
> + if (ret < 0)
> + return ret;
> +
> + pp->root_bus_nr = -1;
> + pp->ops = &tegra_pcie_dw_host_ops;
> +
> + ret = dw_pcie_host_init(pp);
> + if (ret < 0) {
> + dev_err(pcie->dev, "Failed to add PCIe port: %d\n", ret);

Perhaps mention which port? Or is this uniquely identified by
dev_name(pcie->dev) already?

> + goto fail_host_init;
> + }
> +
> + return 0;
> +
> +fail_host_init:
> + return __deinit_controller(pcie);
> +}
> +
> +static int tegra_pcie_try_link_l2(struct tegra_pcie_dw *pcie)
> +{
> + u32 val;
> +
> + if (!tegra_pcie_dw_link_up(&pcie->pci))
> + return 0;
> +
> + val = readl(pcie->appl_base + APPL_RADM_STATUS);
> + val |= APPL_PM_XMT_TURNOFF_STATE;
> + writel(val, pcie->appl_base + APPL_RADM_STATUS);
> +
> + return readl_poll_timeout_atomic(pcie->appl_base + APPL_DEBUG, val,
> + val & APPL_DEBUG_PM_LINKST_IN_L2_LAT,
> + 1, PME_ACK_TIMEOUT);
> +}
> +
> +static void tegra_pcie_dw_pme_turnoff(struct tegra_pcie_dw *pcie)
> +{
> + u32 data;
> + int err;
> +
> + if (!tegra_pcie_dw_link_up(&pcie->pci)) {
> + dev_dbg(pcie->dev, "PCIe link is not up...!\n");
> + return;
> + }
> +
> + if (tegra_pcie_try_link_l2(pcie)) {
> + dev_info(pcie->dev, "Link didn't transition to L2 state\n");
> + /*
> + * TX lane clock freq will reset to Gen1 only if link is in L2
> + * or detect state.
> + * So apply pex_rst to end point to force RP to go into detect
> + * state
> + */
> + data = readl(pcie->appl_base + APPL_PINMUX);
> + data &= ~APPL_PINMUX_PEX_RST;
> + writel(data, pcie->appl_base + APPL_PINMUX);
> +
> + err = readl_poll_timeout_atomic(pcie->appl_base + APPL_DEBUG,
> + data,
> + ((data &
> + APPL_DEBUG_LTSSM_STATE_MASK) >>
> + APPL_DEBUG_LTSSM_STATE_SHIFT) ==
> + LTSSM_STATE_PRE_DETECT,
> + 1, LTSSM_TIMEOUT);
> + if (err) {
> + dev_info(pcie->dev, "Link didn't go to detect state\n");
> + } else {
> + /* Disable LTSSM after link is in detect state */
> + data = readl(pcie->appl_base + APPL_CTRL);
> + data &= ~APPL_CTRL_LTSSM_EN;
> + writel(data, pcie->appl_base + APPL_CTRL);
> + }
> + }
> + /*
> + * DBI registers may not be accessible after this as PLL-E would be
> + * down depending on how CLKREQ is pulled by end point
> + */
> + data = readl(pcie->appl_base + APPL_PINMUX);
> + data |= (APPL_PINMUX_CLKREQ_OVERRIDE_EN | APPL_PINMUX_CLKREQ_OVERRIDE);
> + /* Cut REFCLK to slot */
> + data |= APPL_PINMUX_CLK_OUTPUT_IN_OVERRIDE_EN;
> + data &= ~APPL_PINMUX_CLK_OUTPUT_IN_OVERRIDE;
> + writel(data, pcie->appl_base + APPL_PINMUX);
> +}
> +
> +static int tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie)
> +{
> + /*
> + * link doesn't go into L2 state with some of the endpoints with Tegra
> + * if they are not in D0 state. So, need to make sure that immediate
> + * downstream devices are in D0 state before sending PME_TurnOff to put
> + * link into L2 state
> + */
> + tegra_pcie_downstream_dev_to_D0(pcie);
> + dw_pcie_host_deinit(&pcie->pci.pp);
> + tegra_pcie_dw_pme_turnoff(pcie);
> + return __deinit_controller(pcie);
> +}
> +
> +static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie)
> +{
> + struct pcie_port *pp = &pcie->pci.pp;
> + struct device *dev = pcie->dev;
> + char *name;
> + int ret;
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> + pp->msi_irq = of_irq_get_byname(dev->of_node, "msi");
> + if (!pp->msi_irq) {
> + dev_err(dev, "Failed to get MSI interrupt\n");
> + return -ENODEV;
> + }
> + }
> +
> + pm_runtime_enable(dev);
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0) {
> + dev_err(dev, "Failed to get runtime sync for PCIe dev\n");
> + pm_runtime_disable(dev);
> + return ret;
> + }
> +
> + tegra_pcie_init_controller(pcie);
> +
> + pcie->link_state = tegra_pcie_dw_link_up(&pcie->pci);
> +
> + if (!pcie->link_state) {
> + ret = -ENOMEDIUM;
> + goto fail_host_init;
> + }
> +
> + name = devm_kasprintf(dev, GFP_KERNEL, "%pOFP", dev->of_node);
> + if (!name) {
> + ret = -ENOMEM;
> + goto fail_host_init;
> + }
> +
> + pcie->debugfs = debugfs_create_dir(name, NULL);
> + if (!pcie->debugfs)
> + dev_err(dev, "Failed to create debugfs\n");
> + else
> + init_debugfs(pcie);
> +
> + return ret;
> +
> +fail_host_init:
> + tegra_pcie_deinit_controller(pcie);
> + pm_runtime_put_sync(dev);
> + pm_runtime_disable(dev);
> + return ret;
> +}
> +
> +static const struct tegra_pcie_soc tegra_pcie_rc_of_data = {
> + .mode = DW_PCIE_RC_TYPE,
> +};
> +
> +static const struct of_device_id tegra_pcie_dw_of_match[] = {
> + {
> + .compatible = "nvidia,tegra194-pcie",
> + .data = &tegra_pcie_rc_of_data,
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, tegra_pcie_dw_of_match);
> +
> +static int tegra_pcie_dw_probe(struct platform_device *pdev)
> +{
> + const struct tegra_pcie_soc *data;
> + struct device *dev = &pdev->dev;
> + struct resource *atu_dma_res;
> + struct tegra_pcie_dw *pcie;
> + struct resource *dbi_res;
> + struct pcie_port *pp;
> + struct dw_pcie *pci;
> + struct phy **phys;
> + char *name;
> + int ret;
> + u32 i;
> +
> + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> + if (!pcie)
> + return -ENOMEM;
> +
> + pci = &pcie->pci;
> + pci->dev = &pdev->dev;
> + pci->ops = &tegra_dw_pcie_ops;
> + pp = &pci->pp;
> + pcie->dev = &pdev->dev;
> +
> + data = (struct tegra_pcie_soc *)of_device_get_match_data(dev);
> + if (!data)
> + return -EINVAL;
> + pcie->mode = (enum dw_pcie_device_mode)data->mode;
> +
> + ret = tegra_pcie_dw_parse_dt(pcie);
> + if (ret < 0) {
> + dev_err(pcie->dev, "Failed to parse device tree: %d\n", ret);
> + return ret;
> + }
> +
> + pcie->pex_ctl_supply = devm_regulator_get(dev, "vddio-pex-ctl");
> + if (IS_ERR(pcie->pex_ctl_supply)) {
> + dev_err(dev, "Failed to get regulator: %ld\n",
> + PTR_ERR(pcie->pex_ctl_supply));
> + return PTR_ERR(pcie->pex_ctl_supply);
> + }
> +
> + pcie->core_clk = devm_clk_get(dev, "core");
> + if (IS_ERR(pcie->core_clk)) {
> + dev_err(dev, "Failed to get core clock\n");
> + return PTR_ERR(pcie->core_clk);
> + }
> +
> + pcie->appl_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + "appl");
> + if (!pcie->appl_res) {
> + dev_err(dev, "Failed to find \"appl\" region\n");
> + return PTR_ERR(pcie->appl_res);
> + }
> + pcie->appl_base = devm_ioremap_resource(dev, pcie->appl_res);
> + if (IS_ERR(pcie->appl_base)) {
> + dev_err(dev, "Failed to map \"appl\" region\n");
> + return PTR_ERR(pcie->appl_base);
> + }
> +
> + pcie->core_apb_rst = devm_reset_control_get(pcie->dev, "core_apb");
> + if (IS_ERR(pcie->core_apb_rst)) {
> + dev_err(pcie->dev, "Failed to get core_apb reset\n");
> + return PTR_ERR(pcie->core_apb_rst);
> + }
> +
> + phys = devm_kcalloc(pcie->dev, pcie->phy_count, sizeof(*phys),
> + GFP_KERNEL);
> + if (!phys)
> + return PTR_ERR(phys);
> +
> + for (i = 0; i < pcie->phy_count; i++) {
> + name = kasprintf(GFP_KERNEL, "p2u-%u", i);
> + if (!name) {
> + dev_err(pcie->dev, "Failed to create P2U string\n");
> + return -ENOMEM;
> + }
> + phys[i] = devm_phy_get(pcie->dev, name);
> + kfree(name);
> + if (IS_ERR(phys[i])) {
> + ret = PTR_ERR(phys[i]);
> + dev_err(pcie->dev, "Failed to get phy: %d\n", ret);

s/phy/PHY/

> + return ret;
> + }
> + }
> +
> + pcie->phys = phys;
> +
> + dbi_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> + if (!dbi_res) {
> + dev_err(dev, "Failed to find \"dbi\" region\n");

No need to print this error message...

> + return PTR_ERR(dbi_res);
> + }
> + pcie->dbi_res = dbi_res;
> +
> + pci->dbi_base = devm_ioremap_resource(dev, dbi_res);
> + if (IS_ERR(pci->dbi_base)) {
> + dev_err(dev, "Failed to map \"dbi\" region\n");

... or this one. devm_ioremap_resource() already prints them.

> + return PTR_ERR(pci->dbi_base);
> + }
> +
> + /* Tegra HW locates DBI2 at a fixed offset from DBI */
> + pci->dbi_base2 = pci->dbi_base + 0x1000;
> +
> + atu_dma_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + "atu_dma");
> + if (!atu_dma_res) {
> + dev_err(dev, "Failed to find \"atu_dma\" region\n");
> + return PTR_ERR(atu_dma_res);
> + }
> + pcie->atu_dma_res = atu_dma_res;
> + pci->atu_base = devm_ioremap_resource(dev, atu_dma_res);
> + if (IS_ERR(pci->atu_base)) {
> + dev_err(dev, "Failed to map \"atu_dma\" region\n");
> + return PTR_ERR(pci->atu_base);
> + }
> +
> + pcie->core_rst = devm_reset_control_get(pcie->dev, "core");
> + if (IS_ERR(pcie->core_rst)) {
> + dev_err(pcie->dev, "Failed to get core reset\n");
> + return PTR_ERR(pcie->core_rst);
> + }
> +
> + pp->irq = platform_get_irq_byname(pdev, "intr");
> + if (!pp->irq) {
> + dev_err(pcie->dev, "Failed to get \"intr\" interrupt\n");
> + return -ENODEV;
> + }
> +
> + ret = devm_request_irq(dev, pp->irq, tegra_pcie_irq_handler,
> + IRQF_SHARED, "tegra-pcie-intr", pcie);
> + if (ret) {
> + dev_err(pcie->dev, "Failed to request IRQ %d: %d\n", pp->irq,
> + ret);
> + return ret;
> + }
> +
> + pcie->bpmp = tegra_bpmp_get(pcie->dev);
> + if (IS_ERR(pcie->bpmp))
> + return PTR_ERR(pcie->bpmp);
> +
> + platform_set_drvdata(pdev, pcie);
> +
> + if (pcie->mode == DW_PCIE_RC_TYPE) {
> + ret = tegra_pcie_config_rp(pcie);
> + if (ret && ret != -ENOMEDIUM)
> + goto fail;
> + else
> + return 0;
> + }
> +
> +fail:
> + tegra_bpmp_put(pcie->bpmp);
> + return ret;
> +}
> +
> +static int tegra_pcie_dw_remove(struct platform_device *pdev)
> +{
> + struct tegra_pcie_dw *pcie = platform_get_drvdata(pdev);
> +
> + if (pcie->mode != DW_PCIE_RC_TYPE)
> + return 0;
> +
> + if (!pcie->link_state)
> + return 0;
> +
> + debugfs_remove_recursive(pcie->debugfs);
> + tegra_pcie_deinit_controller(pcie);
> + pm_runtime_put_sync(pcie->dev);
> + pm_runtime_disable(pcie->dev);
> + tegra_bpmp_put(pcie->bpmp);
> +
> + return 0;
> +}
> +
> +static int tegra_pcie_dw_suspend_late(struct device *dev)
> +{
> + struct tegra_pcie_dw *pcie = dev_get_drvdata(dev);
> + u32 val;
> +
> + if (!pcie->link_state)
> + return 0;
> +
> + /* Enable HW_HOT_RST mode */
> + val = readl(pcie->appl_base + APPL_CTRL);
> + val &= ~(APPL_CTRL_HW_HOT_RST_MODE_MASK <<
> + APPL_CTRL_HW_HOT_RST_MODE_SHIFT);
> + val |= APPL_CTRL_HW_HOT_RST_EN;
> + writel(val, pcie->appl_base + APPL_CTRL);
> +
> + return 0;
> +}
> +
> +static int tegra_pcie_dw_suspend_noirq(struct device *dev)
> +{
> + struct tegra_pcie_dw *pcie = dev_get_drvdata(dev);
> +
> + if (!pcie->link_state)
> + return 0;
> +
> + /* Save MSI interrupt vector */
> + pcie->msi_ctrl_int = dw_pcie_readl_dbi(&pcie->pci,
> + PORT_LOGIC_MSI_CTRL_INT_0_EN);
> + tegra_pcie_downstream_dev_to_D0(pcie);
> + tegra_pcie_dw_pme_turnoff(pcie);
> + return __deinit_controller(pcie);
> +}
> +
> +static int tegra_pcie_dw_resume_noirq(struct device *dev)
> +{
> + struct tegra_pcie_dw *pcie = dev_get_drvdata(dev);
> + int ret;
> +
> + if (!pcie->link_state)
> + return 0;
> +
> + ret = tegra_pcie_config_controller(pcie, true);
> + if (ret < 0)
> + return ret;
> +
> + ret = tegra_pcie_dw_host_init(&pcie->pci.pp);
> + if (ret < 0) {
> + dev_err(dev, "Failed to init host: %d\n", ret);
> + goto fail_host_init;
> + }
> +
> + /* Restore MSI interrupt vector */
> + dw_pcie_writel_dbi(&pcie->pci, PORT_LOGIC_MSI_CTRL_INT_0_EN,
> + pcie->msi_ctrl_int);
> +
> + return 0;
> +fail_host_init:
> + return __deinit_controller(pcie);
> +}
> +
> +static int tegra_pcie_dw_resume_early(struct device *dev)
> +{
> + struct tegra_pcie_dw *pcie = dev_get_drvdata(dev);
> + u32 val;
> +
> + if (!pcie->link_state)
> + return 0;
> +
> + /* Disable HW_HOT_RST mode */
> + val = readl(pcie->appl_base + APPL_CTRL);
> + val &= ~(APPL_CTRL_HW_HOT_RST_MODE_MASK <<
> + APPL_CTRL_HW_HOT_RST_MODE_SHIFT);
> + val |= APPL_CTRL_HW_HOT_RST_MODE_IMDT_RST <<
> + APPL_CTRL_HW_HOT_RST_MODE_SHIFT;
> + val &= ~APPL_CTRL_HW_HOT_RST_EN;
> + writel(val, pcie->appl_base + APPL_CTRL);
> +
> + return 0;
> +}
> +
> +static void tegra_pcie_dw_shutdown(struct platform_device *pdev)
> +{
> + struct tegra_pcie_dw *pcie = platform_get_drvdata(pdev);
> +
> + if (pcie->mode != DW_PCIE_RC_TYPE)
> + return;
> +
> + if (!pcie->link_state)
> + return;
> +
> + debugfs_remove_recursive(pcie->debugfs);
> + tegra_pcie_downstream_dev_to_D0(pcie);
> +
> + disable_irq(pcie->pci.pp.irq);
> + if (IS_ENABLED(CONFIG_PCI_MSI))
> + disable_irq(pcie->pci.pp.msi_irq);
> +
> + tegra_pcie_dw_pme_turnoff(pcie);
> + __deinit_controller(pcie);
> +}
> +
> +static const struct dev_pm_ops tegra_pcie_dw_pm_ops = {
> + .suspend_late = tegra_pcie_dw_suspend_late,
> + .suspend_noirq = tegra_pcie_dw_suspend_noirq,
> + .resume_noirq = tegra_pcie_dw_resume_noirq,
> + .resume_early = tegra_pcie_dw_resume_early,
> +};
> +
> +static struct platform_driver tegra_pcie_dw_driver = {
> + .probe = tegra_pcie_dw_probe,
> + .remove = tegra_pcie_dw_remove,
> + .shutdown = tegra_pcie_dw_shutdown,
> + .driver = {
> + .name = "tegra194-pcie",
> +#ifdef CONFIG_PM
> + .pm = &tegra_pcie_dw_pm_ops,
> +#endif

No need for the preprocessor conditional. 64-bit Tegra always has PM
enabled these days.

> + .of_match_table = tegra_pcie_dw_of_match,
> + },
> +};
> +module_platform_driver(tegra_pcie_dw_driver);
> +
> +MODULE_AUTHOR("Vidya Sagar <vidyas@xxxxxxxxxx>");
> +MODULE_DESCRIPTION("NVIDIA PCIe host controller driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.17.1
>

Thierry

Attachment: signature.asc
Description: PGP signature