mbox series

[v6,0/2] PCI: cadence: Retrain Link to work around Gen2

Message ID 20201228140510.14641-1-nadeem@cadence.com
Headers show
Series PCI: cadence: Retrain Link to work around Gen2 | expand

Message

Nadeem Athani Dec. 28, 2020, 2:05 p.m. UTC 
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.
Adding a quirk flag for defective IP. In future IP revisions this will not
be applicable.

Version history:
Changes in v6:
- Move the position of function cdns_pcie_host_wait_for_link to remove
  compilation error. No changes in code. Separate patch for this.
Changes in v5:
- Remove the compatible string based setting of quirk flag.
- Removed additional Link Up Check
- Removed quirk from pcie-cadence-plat.c and added in pci-j721e.c
Changes in v4:
- Added a quirk flag based on a new compatible string.
- Change of api for link up: cdns_pcie_host_wait_for_link().
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.

Nadeem Athani (2):
  PCI: cadence: Retrain Link to work around Gen2 training defect.
  PCI: cadence: Retrain Link to work around Gen2 training defect.

 drivers/pci/controller/cadence/pci-j721e.c         |  3 +
 drivers/pci/controller/cadence/pcie-cadence-host.c | 65 ++++++++++++++++------
 drivers/pci/controller/cadence/pcie-cadence.h      | 11 +++-
 3 files changed, 61 insertions(+), 18 deletions(-)

Comments

Thomas Petazzoni Dec. 29, 2020, 9:31 p.m. UTC | #1 
On Mon, 28 Dec 2020 15:05:10 +0100
Nadeem Athani <nadeem@cadence.com> wrote:

> +static void cdns_pcie_retrain(struct cdns_pcie *pcie)


Shouldn't this propagate a return value ?

> +{

> +	u32 lnk_cap_sls, pcie_cap_off = CDNS_PCIE_RP_CAP_OFFSET;

> +	u16 lnk_stat, lnk_ctl;

> +

> +	/*

> +	 * 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_host_wait_for_link(pcie))

> +			return;


Here, shouldn't you return the status of
cdns_pcie_host_wait_for_link(), to propagate whether the PCIe link
indeed came up after the retrain ?

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com