2021-04-26 05:15:43

by Athani Nadeem Ladkhan

[permalink] [raw]
Subject: [PATCH] PCI: cadence: Set LTSSM Detect Quiet state minimum delay as workaround for training defect.

Adding a quirk flag "quirk_detect_quiet_flag" to program the minimum
time that LTSSM waits on entering Detect.Quiet state.
Setting this to 2ms for specific TI j7200 SOC as a workaround to resolve
a link training issue in IP.
In future revisions this setting will not be required.

As per PCIe specification, all Receivers must meet the Z-RX-DC
specification for 2.5 GT/s within 1ms of entering Detect.Quiet LTSSM
substate. The LTSSM must stay in this substate until the ZRXDC
specification for 2.5 GT/s is met.

00 : 0 minimum wait time in Detect.Quiet state.
01 : 100us minimum wait time in Detect.Quiet state.
10 : 1ms minimum wait time in Detect.Quiet state.
11 : 2ms minimum wait time in Detect.Quiet state.

Signed-off-by: Nadeem Athani <[email protected]>
---
drivers/pci/controller/cadence/pcie-cadence-ep.c | 21 +++++++++++++++++++++
drivers/pci/controller/cadence/pcie-cadence-host.c | 21 +++++++++++++++++++++
drivers/pci/controller/cadence/pcie-cadence.h | 12 ++++++++++++
3 files changed, 54 insertions(+)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
index 897cdde02bd8..245771f03c21 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
@@ -552,6 +552,23 @@ static const struct pci_epc_ops cdns_pcie_epc_ops = {
.get_features = cdns_pcie_ep_get_features,
};

+static void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie_ep *ep)
+{
+ struct cdns_pcie *pcie = &ep->pcie;
+ u32 delay = 0x3;
+ u32 ltssm_control_cap;
+
+ /*
+ * Set the LTSSM Detect Quiet state min. delay to 2ms.
+ */
+
+ ltssm_control_cap = cdns_pcie_readl(pcie, CDNS_PCIE_LTSSM_CONTROL_CAP);
+ ltssm_control_cap = ((ltssm_control_cap &
+ ~CDNS_PCIE_DETECT_QUIET_MIN_DELAY_MASK) |
+ CDNS_PCIE_DETECT_QUIET_MIN_DELAY(delay));
+
+ cdns_pcie_writel(pcie, CDNS_PCIE_LTSSM_CONTROL_CAP, ltssm_control_cap);
+}

int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
{
@@ -623,6 +640,10 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
ep->irq_pci_addr = CDNS_PCIE_EP_IRQ_PCI_ADDR_NONE;
/* Reserve region 0 for IRQs */
set_bit(0, &ep->ob_region_map);
+
+ if (ep->quirk_detect_quiet_flag)
+ cdns_pcie_detect_quiet_min_delay_set(ep);
+
spin_lock_init(&ep->lock);

return 0;
diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
index ae1c55503513..ffc2dbeb1240 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
@@ -462,6 +462,24 @@ static int cdns_pcie_host_init(struct device *dev,
return cdns_pcie_host_init_address_translation(rc);
}

+static void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie_rc *rc)
+{
+ struct cdns_pcie *pcie = &rc->pcie;
+ u32 delay = 0x3;
+ u32 ltssm_control_cap;
+
+ /*
+ * Set the LTSSM Detect Quiet state min. delay to 2ms.
+ */
+
+ ltssm_control_cap = cdns_pcie_readl(pcie, CDNS_PCIE_LTSSM_CONTROL_CAP);
+ ltssm_control_cap = ((ltssm_control_cap &
+ ~CDNS_PCIE_DETECT_QUIET_MIN_DELAY_MASK) |
+ CDNS_PCIE_DETECT_QUIET_MIN_DELAY(delay));
+
+ cdns_pcie_writel(pcie, CDNS_PCIE_LTSSM_CONTROL_CAP, ltssm_control_cap);
+}
+
int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
{
struct device *dev = rc->pcie.dev;
@@ -498,6 +516,9 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
return PTR_ERR(rc->cfg_base);
rc->cfg_res = res;

+ if (rc->quirk_detect_quiet_flag)
+ cdns_pcie_detect_quiet_min_delay_set(rc);
+
ret = cdns_pcie_start_link(pcie);
if (ret) {
dev_err(dev, "Failed to start link\n");
diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
index 254d2570f8c9..d074475fe887 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.h
+++ b/drivers/pci/controller/cadence/pcie-cadence.h
@@ -189,6 +189,14 @@
/* AXI link down register */
#define CDNS_PCIE_AT_LINKDOWN (CDNS_PCIE_AT_BASE + 0x0824)

+/* LTSSM Capabilities register */
+#define CDNS_PCIE_LTSSM_CONTROL_CAP (CDNS_PCIE_LM_BASE + 0x0054)
+#define CDNS_PCIE_DETECT_QUIET_MIN_DELAY_MASK GENMASK(2, 1)
+#define CDNS_PCIE_DETECT_QUIET_MIN_DELAY_SHIFT 1
+#define CDNS_PCIE_DETECT_QUIET_MIN_DELAY(delay) \
+ (((delay) << CDNS_PCIE_DETECT_QUIET_MIN_DELAY_SHIFT) & \
+ CDNS_PCIE_DETECT_QUIET_MIN_DELAY_MASK)
+
enum cdns_pcie_rp_bar {
RP_BAR_UNDEFINED = -1,
RP_BAR0,
@@ -292,6 +300,7 @@ struct cdns_pcie {
* @avail_ib_bar: Satus of RP_BAR0, RP_BAR1 and RP_NO_BAR if it's free or
* available
* @quirk_retrain_flag: Retrain link as quirk for PCIe Gen2
+ * @quirk_detect_quiet_flag: LTSSM Detect Quiet min delay set as quirk
*/
struct cdns_pcie_rc {
struct cdns_pcie pcie;
@@ -301,6 +310,7 @@ struct cdns_pcie_rc {
u32 device_id;
bool avail_ib_bar[CDNS_PCIE_RP_MAX_IB];
bool quirk_retrain_flag;
+ bool quirk_detect_quiet_flag;
};

/**
@@ -331,6 +341,7 @@ struct cdns_pcie_epf {
* registers fields (RMW) accessible by both remote RC and EP to
* minimize time between read and write
* @epf: Structure to hold info about endpoint function
+ * @quirk_detect_quiet_flag: LTSSM Detect Quiet min delay set as quirk
*/
struct cdns_pcie_ep {
struct cdns_pcie pcie;
@@ -345,6 +356,7 @@ struct cdns_pcie_ep {
/* protect writing to PCI_STATUS while raising legacy interrupts */
spinlock_t lock;
struct cdns_pcie_epf *epf;
+ bool quirk_detect_quiet_flag;
};


--
2.15.0


2021-05-05 06:59:47

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH] PCI: cadence: Set LTSSM Detect Quiet state minimum delay as workaround for training defect.

Hi Nadeem,

On 26/04/21 10:44 am, Nadeem Athani wrote:

How about $subject to "PCI: cadence: Add quirk to set maximum possible
wait time in Detect.Quiet state."?
> Adding a quirk flag "quirk_detect_quiet_flag" to program the minimum
> time that LTSSM waits on entering Detect.Quiet state.
> Setting this to 2ms for specific TI j7200 SOC as a workaround to resolve
> a link training issue in IP.

Please add a more detailed description of the issue here. Something like
below.

PCIe fails to link up if SERDES lanes not used by PCIe are assigned to
another protocol. For example, link training fails if lanes 2 and 3 are
assigned to another protocol while lanes 0 and 1 are used for PCIe to
form a two lane link. This failure is due to an incorrect tie-off on an
internal status signal indicating electrical idle.

Status signals going from SERDES to PCIe Controller are tied-off when a
lane is not assigned to PCIe. Signal indicating electrical idle is
incorrectly tied-off to a state that indicates non-idle. As a result,
PCIe sees unused lanes to be out of electrical idle and this causes
LTSSM to exit Detect.Quiet state without waiting for 12ms timeout to
occur. If a receiver is not detected on the first receiver detection
attempt in Detect.Active state, LTSSM goes back to Detect.Quiet and
again moves forward to Detect.Active state without waiting for 12ms as
required by PCIe base specification. Since wait time in Detect.Quiet is
skipped, multiple receiver detect operations are performed back-to-back
without allowing time for capacitance on the transmit lines to
discharge. This causes subsequent receiver detection to always fail even
if a receiver gets connected eventually.
> In future revisions this setting will not be required.
>
> As per PCIe specification, all Receivers must meet the Z-RX-DC
> specification for 2.5 GT/s within 1ms of entering Detect.Quiet LTSSM
> substate. The LTSSM must stay in this substate until the ZRXDC
> specification for 2.5 GT/s is met.
>
> 00 : 0 minimum wait time in Detect.Quiet state.
> 01 : 100us minimum wait time in Detect.Quiet state.
> 10 : 1ms minimum wait time in Detect.Quiet state.
> 11 : 2ms minimum wait time in Detect.Quiet state.
>
> Signed-off-by: Nadeem Athani <[email protected]>
> ---

Please note the previous version of the patch here and what changed.
http://lore.kernel.org/r/[email protected]

> drivers/pci/controller/cadence/pcie-cadence-ep.c | 21 +++++++++++++++++++++
> drivers/pci/controller/cadence/pcie-cadence-host.c | 21 +++++++++++++++++++++
> drivers/pci/controller/cadence/pcie-cadence.h | 12 ++++++++++++
> 3 files changed, 54 insertions(+)
>
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> index 897cdde02bd8..245771f03c21 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> @@ -552,6 +552,23 @@ static const struct pci_epc_ops cdns_pcie_epc_ops = {
> .get_features = cdns_pcie_ep_get_features,
> };
>
> +static void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie_ep *ep)
> +{
> + struct cdns_pcie *pcie = &ep->pcie;
> + u32 delay = 0x3;
> + u32 ltssm_control_cap;
> +
> + /*
> + * Set the LTSSM Detect Quiet state min. delay to 2ms.
> + */
> +
> + ltssm_control_cap = cdns_pcie_readl(pcie, CDNS_PCIE_LTSSM_CONTROL_CAP);
> + ltssm_control_cap = ((ltssm_control_cap &
> + ~CDNS_PCIE_DETECT_QUIET_MIN_DELAY_MASK) |
> + CDNS_PCIE_DETECT_QUIET_MIN_DELAY(delay));
> +
> + cdns_pcie_writel(pcie, CDNS_PCIE_LTSSM_CONTROL_CAP, ltssm_control_cap);
> +}
>
> int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
> {
> @@ -623,6 +640,10 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
> ep->irq_pci_addr = CDNS_PCIE_EP_IRQ_PCI_ADDR_NONE;
> /* Reserve region 0 for IRQs */
> set_bit(0, &ep->ob_region_map);
> +
> + if (ep->quirk_detect_quiet_flag)
> + cdns_pcie_detect_quiet_min_delay_set(ep);
> +
> spin_lock_init(&ep->lock);
>
> return 0;
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
> index ae1c55503513..ffc2dbeb1240 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
> @@ -462,6 +462,24 @@ static int cdns_pcie_host_init(struct device *dev,
> return cdns_pcie_host_init_address_translation(rc);
> }
>
> +static void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie_rc *rc)
> +{
> + struct cdns_pcie *pcie = &rc->pcie;
> + u32 delay = 0x3;
> + u32 ltssm_control_cap;
> +
> + /*
> + * Set the LTSSM Detect Quiet state min. delay to 2ms.
> + */
> +
> + ltssm_control_cap = cdns_pcie_readl(pcie, CDNS_PCIE_LTSSM_CONTROL_CAP);
> + ltssm_control_cap = ((ltssm_control_cap &
> + ~CDNS_PCIE_DETECT_QUIET_MIN_DELAY_MASK) |
> + CDNS_PCIE_DETECT_QUIET_MIN_DELAY(delay));
> +
> + cdns_pcie_writel(pcie, CDNS_PCIE_LTSSM_CONTROL_CAP, ltssm_control_cap);
> +}
> +

Don't repeat this function once for host and once for device. Add this
function pcie-cadence.c and invoked from host and endpoint.

Thanks
Kishon