[V6,06/27] PCI: tegra: Add PCIe Gen2 link speed support
diff mbox series

Message ID 20190618180206.4908-7-mmaddireddy@nvidia.com
State New
Headers show
Series
  • Enable Tegra PCIe root port features
Related show

Commit Message

Manikanta Maddireddy June 18, 2019, 6:01 p.m. UTC
Tegra124, Tegra132, Tegra210 and Tegra186 support Gen2 link speed. After
PCIe link is up in Gen1, set target link speed as Gen2 and retrain link.
Link switches to Gen2 speed if Gen2 capable end point is connected, else
link stays in Gen1.

Per PCIe 4.0r0.9 sec 7.6.3.7 implementation note, driver need to wait for
PCIe LTSSM to come back from recovery before retraining the link.

Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
Acked-by: Thierry Reding <treding@nvidia.com>
---
V6: No change

V5: No change

V4: No change

V3: Added blank line after each while loop.

V2: Changed "for loop" to "while", to make it compact and handled coding
style comments.

 drivers/pci/controller/pci-tegra.c | 64 ++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

Comments

Lorenzo Pieralisi June 20, 2019, 2:32 p.m. UTC | #1
On Tue, Jun 18, 2019 at 11:31:45PM +0530, Manikanta Maddireddy wrote:
> Tegra124, Tegra132, Tegra210 and Tegra186 support Gen2 link speed. After
> PCIe link is up in Gen1, set target link speed as Gen2 and retrain link.
> Link switches to Gen2 speed if Gen2 capable end point is connected, else
> link stays in Gen1.
> 
> Per PCIe 4.0r0.9 sec 7.6.3.7 implementation note, driver need to wait for
> PCIe LTSSM to come back from recovery before retraining the link.
> 
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> Acked-by: Thierry Reding <treding@nvidia.com>
> ---
> V6: No change
> 
> V5: No change
> 
> V4: No change
> 
> V3: Added blank line after each while loop.
> 
> V2: Changed "for loop" to "while", to make it compact and handled coding
> style comments.
> 
>  drivers/pci/controller/pci-tegra.c | 64 ++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index 5e9fcef5f8eb..5d19067f7193 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -191,6 +191,8 @@
>  #define  RP_LINK_CONTROL_STATUS_DL_LINK_ACTIVE	0x20000000
>  #define  RP_LINK_CONTROL_STATUS_LINKSTAT_MASK	0x3fff0000
>  
> +#define RP_LINK_CONTROL_STATUS_2		0x000000b0
> +
>  #define PADS_CTL_SEL		0x0000009c
>  
>  #define PADS_CTL		0x000000a0
> @@ -226,6 +228,7 @@
>  #define PADS_REFCLK_CFG_DRVI_SHIFT		12 /* 15:12 */
>  
>  #define PME_ACK_TIMEOUT 10000
> +#define LINK_RETRAIN_TIMEOUT 100000 /* in usec */
>  
>  struct tegra_msi {
>  	struct msi_controller chip;
> @@ -2089,6 +2092,64 @@ static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port)
>  	return false;
>  }
>  
> +static void tegra_pcie_change_link_speed(struct tegra_pcie *pcie)
> +{
> +	struct device *dev = pcie->dev;
> +	struct tegra_pcie_port *port, *tmp;
> +	ktime_t deadline;
> +	u32 value;
> +
> +	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {

And the reason to use the _safe version is ?

Lorenzo

> +		/*
> +		 * "Supported Link Speeds Vector" in "Link Capabilities 2"
> +		 * is not supported by Tegra. tegra_pcie_change_link_speed()
> +		 * is called only for Tegra chips which support Gen2.
> +		 * So there no harm if supported link speed is not verified.
> +		 */
> +		value = readl(port->base + RP_LINK_CONTROL_STATUS_2);
> +		value &= ~PCI_EXP_LNKSTA_CLS;
> +		value |= PCI_EXP_LNKSTA_CLS_5_0GB;
> +		writel(value, port->base + RP_LINK_CONTROL_STATUS_2);
> +
> +		/*
> +		 * Poll until link comes back from recovery to avoid race
> +		 * condition.
> +		 */
> +		deadline = ktime_add_us(ktime_get(), LINK_RETRAIN_TIMEOUT);
> +
> +		while (ktime_before(ktime_get(), deadline)) {
> +			value = readl(port->base + RP_LINK_CONTROL_STATUS);
> +			if ((value & PCI_EXP_LNKSTA_LT) == 0)
> +				break;
> +
> +			usleep_range(2000, 3000);
> +		}
> +
> +		if (value & PCI_EXP_LNKSTA_LT)
> +			dev_warn(dev, "PCIe port %u link is in recovery\n",
> +				 port->index);
> +
> +		/* Retrain the link */
> +		value = readl(port->base + RP_LINK_CONTROL_STATUS);
> +		value |= PCI_EXP_LNKCTL_RL;
> +		writel(value, port->base + RP_LINK_CONTROL_STATUS);
> +
> +		deadline = ktime_add_us(ktime_get(), LINK_RETRAIN_TIMEOUT);
> +
> +		while (ktime_before(ktime_get(), deadline)) {
> +			value = readl(port->base + RP_LINK_CONTROL_STATUS);
> +			if ((value & PCI_EXP_LNKSTA_LT) == 0)
> +				break;
> +
> +			usleep_range(2000, 3000);
> +		}
> +
> +		if (value & PCI_EXP_LNKSTA_LT)
> +			dev_err(dev, "failed to retrain link of port %u\n",
> +				port->index);
> +	}
> +}
> +
>  static void tegra_pcie_enable_ports(struct tegra_pcie *pcie)
>  {
>  	struct device *dev = pcie->dev;
> @@ -2113,6 +2174,9 @@ static void tegra_pcie_enable_ports(struct tegra_pcie *pcie)
>  		tegra_pcie_port_disable(port);
>  		tegra_pcie_port_free(port);
>  	}
> +
> +	if (pcie->soc->has_gen2)
> +		tegra_pcie_change_link_speed(pcie);
>  }
>  
>  static void tegra_pcie_disable_ports(struct tegra_pcie *pcie)
> -- 
> 2.17.1
>
Manikanta Maddireddy June 20, 2019, 2:57 p.m. UTC | #2
On 20-Jun-19 8:02 PM, Lorenzo Pieralisi wrote:
> On Tue, Jun 18, 2019 at 11:31:45PM +0530, Manikanta Maddireddy wrote:
>> Tegra124, Tegra132, Tegra210 and Tegra186 support Gen2 link speed. After
>> PCIe link is up in Gen1, set target link speed as Gen2 and retrain link.
>> Link switches to Gen2 speed if Gen2 capable end point is connected, else
>> link stays in Gen1.
>>
>> Per PCIe 4.0r0.9 sec 7.6.3.7 implementation note, driver need to wait for
>> PCIe LTSSM to come back from recovery before retraining the link.
>>
>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
>> Acked-by: Thierry Reding <treding@nvidia.com>
>> ---
>> V6: No change
>>
>> V5: No change
>>
>> V4: No change
>>
>> V3: Added blank line after each while loop.
>>
>> V2: Changed "for loop" to "while", to make it compact and handled coding
>> style comments.
>>
>>  drivers/pci/controller/pci-tegra.c | 64 ++++++++++++++++++++++++++++++
>>  1 file changed, 64 insertions(+)
>>
>> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
>> index 5e9fcef5f8eb..5d19067f7193 100644
>> --- a/drivers/pci/controller/pci-tegra.c
>> +++ b/drivers/pci/controller/pci-tegra.c
>> @@ -191,6 +191,8 @@
>>  #define  RP_LINK_CONTROL_STATUS_DL_LINK_ACTIVE	0x20000000
>>  #define  RP_LINK_CONTROL_STATUS_LINKSTAT_MASK	0x3fff0000
>>  
>> +#define RP_LINK_CONTROL_STATUS_2		0x000000b0
>> +
>>  #define PADS_CTL_SEL		0x0000009c
>>  
>>  #define PADS_CTL		0x000000a0
>> @@ -226,6 +228,7 @@
>>  #define PADS_REFCLK_CFG_DRVI_SHIFT		12 /* 15:12 */
>>  
>>  #define PME_ACK_TIMEOUT 10000
>> +#define LINK_RETRAIN_TIMEOUT 100000 /* in usec */
>>  
>>  struct tegra_msi {
>>  	struct msi_controller chip;
>> @@ -2089,6 +2092,64 @@ static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port)
>>  	return false;
>>  }
>>  
>> +static void tegra_pcie_change_link_speed(struct tegra_pcie *pcie)
>> +{
>> +	struct device *dev = pcie->dev;
>> +	struct tegra_pcie_port *port, *tmp;
>> +	ktime_t deadline;
>> +	u32 value;
>> +
>> +	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
> And the reason to use the _safe version is ?
>
> Lorenzo

This function is called in probe and resume_noirq. list entry is deleted in
remove, I don't see any scenario where it can cause a race condition.
It is fine to drop _safe. I will fix it in next version.

Manikanta

>> +		/*
>> +		 * "Supported Link Speeds Vector" in "Link Capabilities 2"
>> +		 * is not supported by Tegra. tegra_pcie_change_link_speed()
>> +		 * is called only for Tegra chips which support Gen2.
>> +		 * So there no harm if supported link speed is not verified.
>> +		 */
>> +		value = readl(port->base + RP_LINK_CONTROL_STATUS_2);
>> +		value &= ~PCI_EXP_LNKSTA_CLS;
>> +		value |= PCI_EXP_LNKSTA_CLS_5_0GB;
>> +		writel(value, port->base + RP_LINK_CONTROL_STATUS_2);
>> +
>> +		/*
>> +		 * Poll until link comes back from recovery to avoid race
>> +		 * condition.
>> +		 */
>> +		deadline = ktime_add_us(ktime_get(), LINK_RETRAIN_TIMEOUT);
>> +
>> +		while (ktime_before(ktime_get(), deadline)) {
>> +			value = readl(port->base + RP_LINK_CONTROL_STATUS);
>> +			if ((value & PCI_EXP_LNKSTA_LT) == 0)
>> +				break;
>> +
>> +			usleep_range(2000, 3000);
>> +		}
>> +
>> +		if (value & PCI_EXP_LNKSTA_LT)
>> +			dev_warn(dev, "PCIe port %u link is in recovery\n",
>> +				 port->index);
>> +
>> +		/* Retrain the link */
>> +		value = readl(port->base + RP_LINK_CONTROL_STATUS);
>> +		value |= PCI_EXP_LNKCTL_RL;
>> +		writel(value, port->base + RP_LINK_CONTROL_STATUS);
>> +
>> +		deadline = ktime_add_us(ktime_get(), LINK_RETRAIN_TIMEOUT);
>> +
>> +		while (ktime_before(ktime_get(), deadline)) {
>> +			value = readl(port->base + RP_LINK_CONTROL_STATUS);
>> +			if ((value & PCI_EXP_LNKSTA_LT) == 0)
>> +				break;
>> +
>> +			usleep_range(2000, 3000);
>> +		}
>> +
>> +		if (value & PCI_EXP_LNKSTA_LT)
>> +			dev_err(dev, "failed to retrain link of port %u\n",
>> +				port->index);
>> +	}
>> +}
>> +
>>  static void tegra_pcie_enable_ports(struct tegra_pcie *pcie)
>>  {
>>  	struct device *dev = pcie->dev;
>> @@ -2113,6 +2174,9 @@ static void tegra_pcie_enable_ports(struct tegra_pcie *pcie)
>>  		tegra_pcie_port_disable(port);
>>  		tegra_pcie_port_free(port);
>>  	}
>> +
>> +	if (pcie->soc->has_gen2)
>> +		tegra_pcie_change_link_speed(pcie);
>>  }
>>  
>>  static void tegra_pcie_disable_ports(struct tegra_pcie *pcie)
>> -- 
>> 2.17.1
>>
Lorenzo Pieralisi June 20, 2019, 3:22 p.m. UTC | #3
On Thu, Jun 20, 2019 at 08:27:15PM +0530, Manikanta Maddireddy wrote:
> 
> 
> On 20-Jun-19 8:02 PM, Lorenzo Pieralisi wrote:
> > On Tue, Jun 18, 2019 at 11:31:45PM +0530, Manikanta Maddireddy wrote:
> >> Tegra124, Tegra132, Tegra210 and Tegra186 support Gen2 link speed. After
> >> PCIe link is up in Gen1, set target link speed as Gen2 and retrain link.
> >> Link switches to Gen2 speed if Gen2 capable end point is connected, else
> >> link stays in Gen1.
> >>
> >> Per PCIe 4.0r0.9 sec 7.6.3.7 implementation note, driver need to wait for
> >> PCIe LTSSM to come back from recovery before retraining the link.
> >>
> >> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> >> Acked-by: Thierry Reding <treding@nvidia.com>
> >> ---
> >> V6: No change
> >>
> >> V5: No change
> >>
> >> V4: No change
> >>
> >> V3: Added blank line after each while loop.
> >>
> >> V2: Changed "for loop" to "while", to make it compact and handled coding
> >> style comments.
> >>
> >>  drivers/pci/controller/pci-tegra.c | 64 ++++++++++++++++++++++++++++++
> >>  1 file changed, 64 insertions(+)
> >>
> >> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> >> index 5e9fcef5f8eb..5d19067f7193 100644
> >> --- a/drivers/pci/controller/pci-tegra.c
> >> +++ b/drivers/pci/controller/pci-tegra.c
> >> @@ -191,6 +191,8 @@
> >>  #define  RP_LINK_CONTROL_STATUS_DL_LINK_ACTIVE	0x20000000
> >>  #define  RP_LINK_CONTROL_STATUS_LINKSTAT_MASK	0x3fff0000
> >>  
> >> +#define RP_LINK_CONTROL_STATUS_2		0x000000b0
> >> +
> >>  #define PADS_CTL_SEL		0x0000009c
> >>  
> >>  #define PADS_CTL		0x000000a0
> >> @@ -226,6 +228,7 @@
> >>  #define PADS_REFCLK_CFG_DRVI_SHIFT		12 /* 15:12 */
> >>  
> >>  #define PME_ACK_TIMEOUT 10000
> >> +#define LINK_RETRAIN_TIMEOUT 100000 /* in usec */
> >>  
> >>  struct tegra_msi {
> >>  	struct msi_controller chip;
> >> @@ -2089,6 +2092,64 @@ static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port)
> >>  	return false;
> >>  }
> >>  
> >> +static void tegra_pcie_change_link_speed(struct tegra_pcie *pcie)
> >> +{
> >> +	struct device *dev = pcie->dev;
> >> +	struct tegra_pcie_port *port, *tmp;
> >> +	ktime_t deadline;
> >> +	u32 value;
> >> +
> >> +	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
> > And the reason to use the _safe version is ?
> >
> > Lorenzo
> 
> This function is called in probe and resume_noirq. list entry is deleted in
> remove, I don't see any scenario where it can cause a race condition.
> It is fine to drop _safe. I will fix it in next version.

I will do it myself, it is not a fundamental issue, do not send another
version.

Thanks,
Lorenzo

Patch
diff mbox series

diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 5e9fcef5f8eb..5d19067f7193 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -191,6 +191,8 @@ 
 #define  RP_LINK_CONTROL_STATUS_DL_LINK_ACTIVE	0x20000000
 #define  RP_LINK_CONTROL_STATUS_LINKSTAT_MASK	0x3fff0000
 
+#define RP_LINK_CONTROL_STATUS_2		0x000000b0
+
 #define PADS_CTL_SEL		0x0000009c
 
 #define PADS_CTL		0x000000a0
@@ -226,6 +228,7 @@ 
 #define PADS_REFCLK_CFG_DRVI_SHIFT		12 /* 15:12 */
 
 #define PME_ACK_TIMEOUT 10000
+#define LINK_RETRAIN_TIMEOUT 100000 /* in usec */
 
 struct tegra_msi {
 	struct msi_controller chip;
@@ -2089,6 +2092,64 @@  static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port)
 	return false;
 }
 
+static void tegra_pcie_change_link_speed(struct tegra_pcie *pcie)
+{
+	struct device *dev = pcie->dev;
+	struct tegra_pcie_port *port, *tmp;
+	ktime_t deadline;
+	u32 value;
+
+	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
+		/*
+		 * "Supported Link Speeds Vector" in "Link Capabilities 2"
+		 * is not supported by Tegra. tegra_pcie_change_link_speed()
+		 * is called only for Tegra chips which support Gen2.
+		 * So there no harm if supported link speed is not verified.
+		 */
+		value = readl(port->base + RP_LINK_CONTROL_STATUS_2);
+		value &= ~PCI_EXP_LNKSTA_CLS;
+		value |= PCI_EXP_LNKSTA_CLS_5_0GB;
+		writel(value, port->base + RP_LINK_CONTROL_STATUS_2);
+
+		/*
+		 * Poll until link comes back from recovery to avoid race
+		 * condition.
+		 */
+		deadline = ktime_add_us(ktime_get(), LINK_RETRAIN_TIMEOUT);
+
+		while (ktime_before(ktime_get(), deadline)) {
+			value = readl(port->base + RP_LINK_CONTROL_STATUS);
+			if ((value & PCI_EXP_LNKSTA_LT) == 0)
+				break;
+
+			usleep_range(2000, 3000);
+		}
+
+		if (value & PCI_EXP_LNKSTA_LT)
+			dev_warn(dev, "PCIe port %u link is in recovery\n",
+				 port->index);
+
+		/* Retrain the link */
+		value = readl(port->base + RP_LINK_CONTROL_STATUS);
+		value |= PCI_EXP_LNKCTL_RL;
+		writel(value, port->base + RP_LINK_CONTROL_STATUS);
+
+		deadline = ktime_add_us(ktime_get(), LINK_RETRAIN_TIMEOUT);
+
+		while (ktime_before(ktime_get(), deadline)) {
+			value = readl(port->base + RP_LINK_CONTROL_STATUS);
+			if ((value & PCI_EXP_LNKSTA_LT) == 0)
+				break;
+
+			usleep_range(2000, 3000);
+		}
+
+		if (value & PCI_EXP_LNKSTA_LT)
+			dev_err(dev, "failed to retrain link of port %u\n",
+				port->index);
+	}
+}
+
 static void tegra_pcie_enable_ports(struct tegra_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
@@ -2113,6 +2174,9 @@  static void tegra_pcie_enable_ports(struct tegra_pcie *pcie)
 		tegra_pcie_port_disable(port);
 		tegra_pcie_port_free(port);
 	}
+
+	if (pcie->soc->has_gen2)
+		tegra_pcie_change_link_speed(pcie);
 }
 
 static void tegra_pcie_disable_ports(struct tegra_pcie *pcie)