linux-kernel - Re: [PATCH v3] PCI: cadence: Retrain Link to work around Gen2 training defect.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d2aa5519-e207-c3e5-9d81-14209d856b54@ti.com>
Date:   Fri, 23 Oct 2020 12:26:52 +0530
From:   Kishon Vijay Abraham I <kishon@...com>
To:     Athani Nadeem Ladkhan <nadeem@...ence.com>,
        "lorenzo.pieralisi@....com" <lorenzo.pieralisi@....com>,
        "robh@...nel.org" <robh@...nel.org>,
        "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Tom Joseph <tjoseph@...ence.com>
CC:     Swapnil Kashinath Jakhade <sjakhade@...ence.com>,
        Milind Parab <mparab@...ence.com>
Subject: Re: [PATCH v3] PCI: cadence: Retrain Link to work around Gen2
 training defect.

Hi Nadeem,

On 19/10/20 10:48 pm, Athani Nadeem Ladkhan wrote:
> Hi Kishon,
> 
>> -----Original Message-----
>> From: Kishon Vijay Abraham I <kishon@...com>
>> Sent: Monday, October 19, 2020 10:59 AM
>> To: Athani Nadeem Ladkhan <nadeem@...ence.com>;
>> lorenzo.pieralisi@....com; robh@...nel.org; bhelgaas@...gle.com; linux-
>> pci@...r.kernel.org; linux-kernel@...r.kernel.org; Tom Joseph
>> <tjoseph@...ence.com>
>> Cc: Swapnil Kashinath Jakhade <sjakhade@...ence.com>; Milind Parab
>> <mparab@...ence.com>
>> Subject: Re: [PATCH v3] PCI: cadence: Retrain Link to work around Gen2
>> training defect.
>>
>> EXTERNAL MAIL
>>
>>
>> Hi Nadeem,
>>
>> On 30/09/20 11:51 pm, Nadeem Athani wrote:
>>> Cadence controller will not initiate autonomous speed change if
>>> strapped as Gen2. The Retrain Link bit is set as quirk to enable this speed
>> change.
>>>
>>> Signed-off-by: Nadeem Athani <nadeem@...ence.com>
>>> ---
>>> Changes in v3:
>>> - To set retrain link bit,checking device capability & link status.
>>> - 32bit read in place of 8bit.
>>> - Minor correction in patch comment.
>>> - Change in variable & macro name.
>>> Changes in v2:
>>> - 16bit read in place of 8bit.
>>>  drivers/pci/controller/cadence/pcie-cadence-host.c | 31
>> ++++++++++++++++++++++
>>>  drivers/pci/controller/cadence/pcie-cadence.h      |  9 ++++++-
>>>  2 files changed, 39 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c
>>> b/drivers/pci/controller/cadence/pcie-cadence-host.c
>>> index 4550e0d469ca..2b2ae4e18032 100644
>>> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
>>> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
>>> @@ -77,6 +77,36 @@ static struct pci_ops cdns_pcie_host_ops = {
>>>  	.write		= pci_generic_config_write,
>>>  };
>>>
>>> +static void cdns_pcie_retrain(struct cdns_pcie *pcie) {
>>> +	u32 lnk_cap_sls, pcie_cap_off = CDNS_PCIE_RP_CAP_OFFSET;
>>> +	u16 lnk_stat, lnk_ctl;
>>> +
>>> +	if (!cdns_pcie_link_up(pcie))
>>> +		return;
>>> +
>>
>> Is there a IP version that can be used to check if that quirk is applicable?
> There is no such provision.

hmm okay. Can we add a DT property to indicate the quirk then since
AFAIK this is not required in future revisions of IP.
>>> +	/*
>>> +	 * Set retrain bit if current speed is 2.5 GB/s,
>>> +	 * but the PCIe root port support is > 2.5 GB/s.
>>> +	 */
>>> +
>>> +	lnk_cap_sls = cdns_pcie_readl(pcie, (CDNS_PCIE_RP_BASE +
>> pcie_cap_off +
>>> +				      PCI_EXP_LNKCAP));
>>> +	if ((lnk_cap_sls & PCI_EXP_LNKCAP_SLS) <=
>> PCI_EXP_LNKCAP_SLS_2_5GB)
>>> +		return;
>>> +
>>> +	lnk_stat = cdns_pcie_rp_readw(pcie, pcie_cap_off +
>> PCI_EXP_LNKSTA);
>>> +	if ((lnk_stat & PCI_EXP_LNKSTA_CLS) == PCI_EXP_LNKSTA_CLS_2_5GB)
>> {
>>> +		lnk_ctl = cdns_pcie_rp_readw(pcie,
>>> +					     pcie_cap_off + PCI_EXP_LNKCTL);
>>> +		lnk_ctl |= PCI_EXP_LNKCTL_RL;
>>> +		cdns_pcie_rp_writew(pcie, pcie_cap_off + PCI_EXP_LNKCTL,
>>> +				    lnk_ctl);
>>> +
>>> +		if (!cdns_pcie_link_up(pcie))
>>
>> Should this rather be a cdns_pcie_host_wait_for_link()?
> The use of this api cdns_pcie_link_up was mentioned in earlier reviews.
> The mentioned api cdns_pcie_host_wait_for_link is a wrapper in which there are multiple checks.
> If insist, will replace with it.

yeah, I think we should give some time for the link up to succeed after
re-training.

Thanks
Kishon