[PATCH V2 0/4] Enhancements to Tegra194 PCIe driver

[PATCH V2 0/4] Enhancements to Tegra194 PCIe driver

[Vidya Sagar]

This series of patches do some enhancements and some bug fixes to the
Tegra194 PCIe platform driver like
- Fixing Vendor-ID corruption
- Mapping DBI space correctly
- Updating DWC IP version
- Handling error conditions properly

V2;
* Addressed Rob's comments. Changed 'Strongly Ordered' to 'nGnRnE'

Vidya Sagar (4):
  PCI: tegra: Fix ASPM-L1SS advertisement disable code
  PCI: tegra: Map configuration space as nGnRnE
  PCI: tegra: Set DesignWare IP version
  PCI: tegra: Handle error conditions properly

 drivers/pci/controller/dwc/pcie-tegra194.c | 62 +++++++++++-----------
 1 file changed, 30 insertions(+), 32 deletions(-)

-- 
2.17.1

[PATCH V2 1/4] PCI: tegra: Fix ASPM-L1SS advertisement disable code

[Vidya Sagar]

If the absence of CLKREQ# signal is indicated by the absence of
"supports-clkreq" in the device-tree node, current driver is disabling
the advertisement of ASPM-L1 Sub-States *before* the ASPM-L1 Sub-States
offset is correctly initialized. Since default value of the ASPM-L1SS
offset is zero, this is causing the Vendor-ID wrongly programmed to 0x10d2
instead of Nvidia's 0x10de thereby the quirks applicable for Tegra194 are
not being applied. This patch fixes this issue by refactoring the
code that disables the ASPM-L1SS advertisement.

Fixes: 56e15a238d92 ("PCI: tegra: Add Tegra194 PCIe support")
Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
V2:
* None

 drivers/pci/controller/dwc/pcie-tegra194.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index aa511ec0d800..b172b1d49713 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -896,6 +896,12 @@ static void tegra_pcie_prepare_host(struct pcie_port *pp)
 
 	init_host_aspm(pcie);
 
+	/* Disable ASPM-L1SS advertisement if there is no CLKREQ routing */
+	if (!pcie->supports_clkreq) {
+		disable_aspm_l11(pcie);
+		disable_aspm_l12(pcie);
+	}
+
 	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);
@@ -1400,12 +1406,6 @@ static int tegra_pcie_config_controller(struct tegra_pcie_dw *pcie,
 	pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci,
 						      PCI_CAP_ID_EXP);
 
-	/* Disable ASPM-L1SS advertisement as there is no CLKREQ routing */
-	if (!pcie->supports_clkreq) {
-		disable_aspm_l11(pcie);
-		disable_aspm_l12(pcie);
-	}
-
 	return ret;
 
 fail_phy:
-- 
2.17.1

[PATCH V2 2/4] PCI: tegra: Map configuration space as nGnRnE

[Vidya Sagar]

As specified in the comment for pci_remap_cfgspace() define in
arch/arm64/include/asm/io.h file, PCIe configuration space should be
mapped as nGnRnE. Hence changing to dev_pci_remap_cfgspace() from
devm_ioremap_resource() for mapping DBI space as that is nothing but
the root port's own configuration space.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
V2:
* Changed 'Strongly Ordered' to 'nGnRnE'

 drivers/pci/controller/dwc/pcie-tegra194.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index b172b1d49713..7a0c64436861 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -2108,7 +2108,9 @@ static int tegra_pcie_dw_probe(struct platform_device *pdev)
 	}
 	pcie->dbi_res = dbi_res;
 
-	pci->dbi_base = devm_ioremap_resource(dev, dbi_res);
+	pci->dbi_base = devm_pci_remap_cfgspace(dev,
+						dbi_res->start,
+						resource_size(dbi_res));
 	if (IS_ERR(pci->dbi_base))
 		return PTR_ERR(pci->dbi_base);
 
-- 
2.17.1

[PATCH V2 3/4] PCI: tegra: Set DesignWare IP version

[Vidya Sagar]

Set the DesignWare IP version for Tegra194 to 0x490A. This would be used
by the DesigWare sub-system to do any version specific configuration
(Ex:- TD bit programming for ECRC).

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
V2:
* None

 drivers/pci/controller/dwc/pcie-tegra194.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 7a0c64436861..253d91033bc3 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -2011,6 +2011,7 @@ static int tegra_pcie_dw_probe(struct platform_device *pdev)
 	pci->ops = &tegra_dw_pcie_ops;
 	pci->n_fts[0] = N_FTS_VAL;
 	pci->n_fts[1] = FTS_VAL;
+	pci->version = 0x490A;
 
 	pp = &pci->pp;
 	pcie->dev = &pdev->dev;
-- 
2.17.1

[PATCH V2 4/4] PCI: tegra: Handle error conditions properly

[Vidya Sagar]

Currently the driver checks for error value of different APIs during the
uninitialization sequence. It just returns from there if there is any error
observed for one of those calls. Comparatively it is better to continue the
uninitialization sequence irrespective of whether some of them are
returning error. That way, it is more closer to complete uninitialization.
It also adds checking return value for error for a cleaner exit path.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
V2:
* None

 drivers/pci/controller/dwc/pcie-tegra194.c | 45 ++++++++++------------
 1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 253d91033bc3..8c08998b9ce1 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1422,43 +1422,32 @@ static int tegra_pcie_config_controller(struct tegra_pcie_dw *pcie,
 	return ret;
 }
 
-static int __deinit_controller(struct tegra_pcie_dw *pcie)
+static void tegra_pcie_unconfig_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;
-	}
+	if (ret)
+		dev_err(pcie->dev, "Failed to assert \"core\" reset: %d\n", ret);
 
 	tegra_pcie_disable_phy(pcie);
 
 	ret = reset_control_assert(pcie->core_apb_rst);
-	if (ret) {
+	if (ret)
 		dev_err(pcie->dev, "Failed to assert APB reset: %d\n", ret);
-		return ret;
-	}
 
 	clk_disable_unprepare(pcie->core_clk);
 
 	ret = regulator_disable(pcie->pex_ctl_supply);
-	if (ret) {
+	if (ret)
 		dev_err(pcie->dev, "Failed to disable regulator: %d\n", ret);
-		return ret;
-	}
 
 	tegra_pcie_disable_slot_regulators(pcie);
 
 	ret = tegra_pcie_bpmp_set_ctrl_state(pcie, false);
-	if (ret) {
+	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)
@@ -1482,7 +1471,8 @@ static int tegra_pcie_init_controller(struct tegra_pcie_dw *pcie)
 	return 0;
 
 fail_host_init:
-	return __deinit_controller(pcie);
+	tegra_pcie_unconfig_controller(pcie);
+	return ret;
 }
 
 static int tegra_pcie_try_link_l2(struct tegra_pcie_dw *pcie)
@@ -1551,13 +1541,12 @@ static void tegra_pcie_dw_pme_turnoff(struct tegra_pcie_dw *pcie)
 	appl_writel(pcie, data, APPL_PINMUX);
 }
 
-static int tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie)
+static void tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie)
 {
 	tegra_pcie_downstream_dev_to_D0(pcie);
 	dw_pcie_host_deinit(&pcie->pci.pp);
 	tegra_pcie_dw_pme_turnoff(pcie);
-
-	return __deinit_controller(pcie);
+	tegra_pcie_unconfig_controller(pcie);
 }
 
 static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie)
@@ -1590,7 +1579,11 @@ static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie)
 		goto fail_pm_get_sync;
 	}
 
-	tegra_pcie_init_controller(pcie);
+	ret = tegra_pcie_init_controller(pcie);
+	if (ret < 0) {
+		dev_err(dev, "Failed to initialize controller: %d\n", ret);
+		goto fail_pm_get_sync;
+	}
 
 	pcie->link_state = tegra_pcie_dw_link_up(&pcie->pci);
 	if (!pcie->link_state) {
@@ -2238,8 +2231,9 @@ static int tegra_pcie_dw_suspend_noirq(struct device *dev)
 					       PORT_LOGIC_MSI_CTRL_INT_0_EN);
 	tegra_pcie_downstream_dev_to_D0(pcie);
 	tegra_pcie_dw_pme_turnoff(pcie);
+	tegra_pcie_unconfig_controller(pcie);
 
-	return __deinit_controller(pcie);
+	return 0;
 }
 
 static int tegra_pcie_dw_resume_noirq(struct device *dev)
@@ -2267,7 +2261,8 @@ static int tegra_pcie_dw_resume_noirq(struct device *dev)
 	return 0;
 
 fail_host_init:
-	return __deinit_controller(pcie);
+	tegra_pcie_unconfig_controller(pcie);
+	return ret;
 }
 
 static int tegra_pcie_dw_resume_early(struct device *dev)
@@ -2305,7 +2300,7 @@ static void tegra_pcie_dw_shutdown(struct platform_device *pdev)
 		disable_irq(pcie->pci.pp.msi_irq);
 
 	tegra_pcie_dw_pme_turnoff(pcie);
-	__deinit_controller(pcie);
+	tegra_pcie_unconfig_controller(pcie);
 }
 
 static const struct tegra_pcie_dw_of_data tegra_pcie_dw_rc_of_data = {
-- 
2.17.1

Re: [PATCH V2 4/4] PCI: tegra: Handle error conditions properly

[Bjorn Helgaas]

Hi Vidya,

Can you update the subject to replace "properly" with more details
about what the patch is doing?  "Properly" is really meaningless in
usages like this -- nobody writes patches to do the *wrong* thing, so
it goes without saying that every patch is intended to things
"properly".

It would also help to have some context.  My first thought was that
"error conditions" referred to PCIe errors like completion timeouts,
completer abort, etc.

Maybe something like:

  PCI: tegra: Continue unconfig sequence even if parts fail
  PCI: tegra: Return init error (not unconfig error) on init failure

On Thu, Oct 29, 2020 at 10:48:39AM +0530, Vidya Sagar wrote:
> Currently the driver checks for error value of different APIs during the
> uninitialization sequence. It just returns from there if there is any error
> observed for one of those calls. Comparatively it is better to continue the
> uninitialization sequence irrespective of whether some of them are
> returning error. That way, it is more closer to complete uninitialization.
> It also adds checking return value for error for a cleaner exit path.

This paragraph uses "it" to refer to both "the driver" (second
sentence) and "this patch" (last sentence).  That's confusing.
There's no reason to refer to "this patch" at all.  I'd rather have
"Add checking ..." than "It adds checking ..."

I think that last sentence must be referring to the
tegra_pcie_init_controller() change to return the initialization error
rather than the error from __deinit_controller().  That seems right,
but should be a separate patch.

> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> V2:
> * None
> 
>  drivers/pci/controller/dwc/pcie-tegra194.c | 45 ++++++++++------------
>  1 file changed, 20 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 253d91033bc3..8c08998b9ce1 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1422,43 +1422,32 @@ static int tegra_pcie_config_controller(struct tegra_pcie_dw *pcie,
>  	return ret;
>  }
>  
> -static int __deinit_controller(struct tegra_pcie_dw *pcie)
> +static void tegra_pcie_unconfig_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;
> -	}
> +	if (ret)
> +		dev_err(pcie->dev, "Failed to assert \"core\" reset: %d\n", ret);
>  
>  	tegra_pcie_disable_phy(pcie);
>  
>  	ret = reset_control_assert(pcie->core_apb_rst);
> -	if (ret) {
> +	if (ret)
>  		dev_err(pcie->dev, "Failed to assert APB reset: %d\n", ret);
> -		return ret;
> -	}
>  
>  	clk_disable_unprepare(pcie->core_clk);
>  
>  	ret = regulator_disable(pcie->pex_ctl_supply);
> -	if (ret) {
> +	if (ret)
>  		dev_err(pcie->dev, "Failed to disable regulator: %d\n", ret);
> -		return ret;
> -	}
>  
>  	tegra_pcie_disable_slot_regulators(pcie);
>  
>  	ret = tegra_pcie_bpmp_set_ctrl_state(pcie, false);
> -	if (ret) {
> +	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)
> @@ -1482,7 +1471,8 @@ static int tegra_pcie_init_controller(struct tegra_pcie_dw *pcie)
>  	return 0;
>  
>  fail_host_init:
> -	return __deinit_controller(pcie);
> +	tegra_pcie_unconfig_controller(pcie);
> +	return ret;
>  }
>  
>  static int tegra_pcie_try_link_l2(struct tegra_pcie_dw *pcie)
> @@ -1551,13 +1541,12 @@ static void tegra_pcie_dw_pme_turnoff(struct tegra_pcie_dw *pcie)
>  	appl_writel(pcie, data, APPL_PINMUX);
>  }
>  
> -static int tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie)
> +static void tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie)
>  {
>  	tegra_pcie_downstream_dev_to_D0(pcie);
>  	dw_pcie_host_deinit(&pcie->pci.pp);
>  	tegra_pcie_dw_pme_turnoff(pcie);
> -
> -	return __deinit_controller(pcie);
> +	tegra_pcie_unconfig_controller(pcie);
>  }
>  
>  static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie)
> @@ -1590,7 +1579,11 @@ static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie)
>  		goto fail_pm_get_sync;
>  	}
>  
> -	tegra_pcie_init_controller(pcie);
> +	ret = tegra_pcie_init_controller(pcie);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to initialize controller: %d\n", ret);
> +		goto fail_pm_get_sync;
> +	}
>  
>  	pcie->link_state = tegra_pcie_dw_link_up(&pcie->pci);
>  	if (!pcie->link_state) {
> @@ -2238,8 +2231,9 @@ static int tegra_pcie_dw_suspend_noirq(struct device *dev)
>  					       PORT_LOGIC_MSI_CTRL_INT_0_EN);
>  	tegra_pcie_downstream_dev_to_D0(pcie);
>  	tegra_pcie_dw_pme_turnoff(pcie);
> +	tegra_pcie_unconfig_controller(pcie);
>  
> -	return __deinit_controller(pcie);
> +	return 0;
>  }
>  
>  static int tegra_pcie_dw_resume_noirq(struct device *dev)
> @@ -2267,7 +2261,8 @@ static int tegra_pcie_dw_resume_noirq(struct device *dev)
>  	return 0;
>  
>  fail_host_init:
> -	return __deinit_controller(pcie);
> +	tegra_pcie_unconfig_controller(pcie);
> +	return ret;
>  }
>  
>  static int tegra_pcie_dw_resume_early(struct device *dev)
> @@ -2305,7 +2300,7 @@ static void tegra_pcie_dw_shutdown(struct platform_device *pdev)
>  		disable_irq(pcie->pci.pp.msi_irq);
>  
>  	tegra_pcie_dw_pme_turnoff(pcie);
> -	__deinit_controller(pcie);
> +	tegra_pcie_unconfig_controller(pcie);
>  }
>  
>  static const struct tegra_pcie_dw_of_data tegra_pcie_dw_rc_of_data = {
> -- 
> 2.17.1
> 

Re: [PATCH V2 4/4] PCI: tegra: Handle error conditions properly

[Vidya Sagar]

On 11/4/2020 2:18 AM, Bjorn Helgaas wrote:
> External email: Use caution opening links or attachments
> 
> 
> Hi Vidya,
> 
> Can you update the subject to replace "properly" with more details
> about what the patch is doing?  "Properly" is really meaningless in
> usages like this -- nobody writes patches to do the *wrong* thing, so
> it goes without saying that every patch is intended to things
> "properly".
> 
> It would also help to have some context.  My first thought was that
> "error conditions" referred to PCIe errors like completion timeouts,
> completer abort, etc.
> 
> Maybe something like:
> 
>    PCI: tegra: Continue unconfig sequence even if parts fail
Thanks for reviewing the change.
Sure. I'll go with the above subject line.

>    PCI: tegra: Return init error (not unconfig error) on init failure
> 
> On Thu, Oct 29, 2020 at 10:48:39AM +0530, Vidya Sagar wrote:
>> Currently the driver checks for error value of different APIs during the
>> uninitialization sequence. It just returns from there if there is any error
>> observed for one of those calls. Comparatively it is better to continue the
>> uninitialization sequence irrespective of whether some of them are
>> returning error. That way, it is more closer to complete uninitialization.
>> It also adds checking return value for error for a cleaner exit path.
> 
> This paragraph uses "it" to refer to both "the driver" (second
> sentence) and "this patch" (last sentence).  That's confusing.
> There's no reason to refer to "this patch" at all.  I'd rather have
> "Add checking ..." than "It adds checking ..."
> 
> I think that last sentence must be referring to the
> tegra_pcie_init_controller() change to return the initialization error
> rather than the error from __deinit_controller().  That seems right,
> but should be a separate patch.
Sure. I'll push a new patch for this.

> 
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>> V2:
>> * None
>>
>>   drivers/pci/controller/dwc/pcie-tegra194.c | 45 ++++++++++------------
>>   1 file changed, 20 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
>> index 253d91033bc3..8c08998b9ce1 100644
>> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
>> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
>> @@ -1422,43 +1422,32 @@ static int tegra_pcie_config_controller(struct tegra_pcie_dw *pcie,
>>        return ret;
>>   }
>>
>> -static int __deinit_controller(struct tegra_pcie_dw *pcie)
>> +static void tegra_pcie_unconfig_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;
>> -     }
>> +     if (ret)
>> +             dev_err(pcie->dev, "Failed to assert \"core\" reset: %d\n", ret);
>>
>>        tegra_pcie_disable_phy(pcie);
>>
>>        ret = reset_control_assert(pcie->core_apb_rst);
>> -     if (ret) {
>> +     if (ret)
>>                dev_err(pcie->dev, "Failed to assert APB reset: %d\n", ret);
>> -             return ret;
>> -     }
>>
>>        clk_disable_unprepare(pcie->core_clk);
>>
>>        ret = regulator_disable(pcie->pex_ctl_supply);
>> -     if (ret) {
>> +     if (ret)
>>                dev_err(pcie->dev, "Failed to disable regulator: %d\n", ret);
>> -             return ret;
>> -     }
>>
>>        tegra_pcie_disable_slot_regulators(pcie);
>>
>>        ret = tegra_pcie_bpmp_set_ctrl_state(pcie, false);
>> -     if (ret) {
>> +     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)
>> @@ -1482,7 +1471,8 @@ static int tegra_pcie_init_controller(struct tegra_pcie_dw *pcie)
>>        return 0;
>>
>>   fail_host_init:
>> -     return __deinit_controller(pcie);
>> +     tegra_pcie_unconfig_controller(pcie);
>> +     return ret;
>>   }
>>
>>   static int tegra_pcie_try_link_l2(struct tegra_pcie_dw *pcie)
>> @@ -1551,13 +1541,12 @@ static void tegra_pcie_dw_pme_turnoff(struct tegra_pcie_dw *pcie)
>>        appl_writel(pcie, data, APPL_PINMUX);
>>   }
>>
>> -static int tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie)
>> +static void tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie)
>>   {
>>        tegra_pcie_downstream_dev_to_D0(pcie);
>>        dw_pcie_host_deinit(&pcie->pci.pp);
>>        tegra_pcie_dw_pme_turnoff(pcie);
>> -
>> -     return __deinit_controller(pcie);
>> +     tegra_pcie_unconfig_controller(pcie);
>>   }
>>
>>   static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie)
>> @@ -1590,7 +1579,11 @@ static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie)
>>                goto fail_pm_get_sync;
>>        }
>>
>> -     tegra_pcie_init_controller(pcie);
>> +     ret = tegra_pcie_init_controller(pcie);
>> +     if (ret < 0) {
>> +             dev_err(dev, "Failed to initialize controller: %d\n", ret);
>> +             goto fail_pm_get_sync;
>> +     }
>>
>>        pcie->link_state = tegra_pcie_dw_link_up(&pcie->pci);
>>        if (!pcie->link_state) {
>> @@ -2238,8 +2231,9 @@ static int tegra_pcie_dw_suspend_noirq(struct device *dev)
>>                                               PORT_LOGIC_MSI_CTRL_INT_0_EN);
>>        tegra_pcie_downstream_dev_to_D0(pcie);
>>        tegra_pcie_dw_pme_turnoff(pcie);
>> +     tegra_pcie_unconfig_controller(pcie);
>>
>> -     return __deinit_controller(pcie);
>> +     return 0;
>>   }
>>
>>   static int tegra_pcie_dw_resume_noirq(struct device *dev)
>> @@ -2267,7 +2261,8 @@ static int tegra_pcie_dw_resume_noirq(struct device *dev)
>>        return 0;
>>
>>   fail_host_init:
>> -     return __deinit_controller(pcie);
>> +     tegra_pcie_unconfig_controller(pcie);
>> +     return ret;
>>   }
>>
>>   static int tegra_pcie_dw_resume_early(struct device *dev)
>> @@ -2305,7 +2300,7 @@ static void tegra_pcie_dw_shutdown(struct platform_device *pdev)
>>                disable_irq(pcie->pci.pp.msi_irq);
>>
>>        tegra_pcie_dw_pme_turnoff(pcie);
>> -     __deinit_controller(pcie);
>> +     tegra_pcie_unconfig_controller(pcie);
>>   }
>>
>>   static const struct tegra_pcie_dw_of_data tegra_pcie_dw_rc_of_data = {
>> --
>> 2.17.1
>>