On 12-Dec-17 8:02 PM, Lorenzo Pieralisi wrote:
> [+Ley Foon Tan]
>
> On Mon, Oct 30, 2017 at 07:27:14PM +0530, Manikanta Maddireddy wrote:
>> Tegra124, 132, 210 and 186 support Gen2 link speed. After the 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.
>>
>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx>
>> ---
>> V3:
>> * Corrected commit log
>> * Replaced jiffies with ktime
>> V2:
>> * no change in this patch
>>
>> drivers/pci/host/pci-tegra.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 42 insertions(+)
>>
>> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
>> index 068510b40c1a..ed5e8acfdc32 100644
>> --- a/drivers/pci/host/pci-tegra.c
>> +++ b/drivers/pci/host/pci-tegra.c
>> @@ -232,6 +232,8 @@
>> #define PADS_REFCLK_CFG_PREDI_SHIFT 8 /* 11:8 */
>> #define PADS_REFCLK_CFG_DRVI_SHIFT 12 /* 15:12 */
>>
>> +#define LINK_RETRAIN_TIMEOUT 100000
>> +
>> struct tegra_msi {
>> struct msi_controller chip;
>> DECLARE_BITMAP(used, INT_PCI_MSI_NR);
>> @@ -2134,6 +2136,42 @@ static void tegra_pcie_enable_ports(struct tegra_pcie *pcie)
>> }
>> }
>>
>> +static void tegra_pcie_change_link_speed(struct tegra_pcie *pcie,
>> + struct pci_dev *pci_dev)
>> +{
>> + struct device *dev = pcie->dev;
>> + ktime_t deadline;
>> + unsigned short val;
>
> u16
>
>> + /* Skip if the current device is not a root port */
>> + if (pci_pcie_type(pci_dev) != PCI_EXP_TYPE_ROOT_PORT)
>> + return;
>> +
>> + pcie_capability_read_word(pci_dev, PCI_EXP_LNKCTL2, &val);
>> + val &= ~PCI_EXP_LNKSTA_CLS;
>> + val |= PCI_EXP_LNKSTA_CLS_5_0GB;
>> + pcie_capability_write_word(pci_dev, PCI_EXP_LNKCTL2, val);
>
> Should you not read the Link Capabilities 2 register ("Supported Speed
> Vector") before programming the Link control 2 register Target Link
> Speed value ?
>
Link Capabilities 2 register is hardwired to 0 and not used in Tegra.
This information is documented in Tegra TRM.
>> +
>> + /* Retrain the link */
>> + pcie_capability_read_word(pci_dev, PCI_EXP_LNKCTL, &val);
>> + val |= PCI_EXP_LNKCTL_RL;
>> + pcie_capability_write_word(pci_dev, PCI_EXP_LNKCTL, val);
>> +
>> + deadline = ktime_add_us(ktime_get(), LINK_RETRAIN_TIMEOUT);
>> + for (;;) {
>> + pcie_capability_read_word(pci_dev, PCI_EXP_LNKSTA, &val);
>> + if (!(val & PCI_EXP_LNKSTA_LT))
>> + break;
>> + if (ktime_after(ktime_get(), deadline))
>> + break;
>> + usleep_range(2000, 3000);
>
> Ok - I hope we won't end up with every host bridge re-writing its own
> link training loop because at that point in time we should think about
> consolidating this.
>
Are you saying that we need to add common link retrain function in
pci core driver and reuse it in all host drivers?
> CC'ing Ley Foon Tan since I would like to understand why the Altera
> driver link retraining can't be written with the same code as this
> driver - I suspect it has to do with the retraining sequence and when
> the retraining is actually carried out in the host bridge probe
> sequence.
>
>> + }
>> +
>> + if (val & PCI_EXP_LNKSTA_LT)
>> + dev_err(dev, "link retrain of PCIe slot %u failed\n",
>> + PCI_SLOT(pci_dev->devfn));
>> +}
>> +
>> static const struct tegra_pcie_soc tegra20_pcie = {
>> .num_ports = 2,
>> .msi_base_shift = 0,
>> @@ -2335,6 +2373,7 @@ static int tegra_pcie_probe(struct platform_device *pdev)
>> struct pci_host_bridge *host;
>> struct tegra_pcie *pcie;
>> struct pci_bus *child;
>> + struct pci_dev *pci_dev = NULL;
>> int err;
>>
>> host = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
>> @@ -2400,6 +2439,9 @@ static int tegra_pcie_probe(struct platform_device *pdev)
>>
>> pci_bus_add_devices(host->bus);
>>
>> + for_each_pci_dev(pci_dev)
>> + tegra_pcie_change_link_speed(pcie, pci_dev);
>> +
>
> Are you sure it is safe to change link speed after adding devices ?
>
> Lorenzo
I tried to do link retrain right after 'linkup in Gen1' i.e before pci_bus_add_devices(),
but it taking more time than timeout(100 msec) I added in tegra_pcie_change_link_speed().
So I moved it here to have minimum delay for retraining link. I didn't see any issue
here, link speed is moving to Gen2 without any issue. Do you want me look into anything
particular here?
>
>> if (IS_ENABLED(CONFIG_DEBUG_FS)) {
>> err = tegra_pcie_debugfs_init(pcie);
>> if (err < 0)
>> --
>> 2.1.4
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
