Re: [”PATCH” v2 1/5] PCI: armada8k: Disable LTSSM on link down interrupts

["Pali Rohár" <pali@kernel.org>]

On Wednesday 14 April 2021 16:20:50 bpeled@marvell.com wrote:
> From: Ben Peled <bpeled@marvell.com>
> 
> When a PCI link down condition is detected, the link training state
> machine must be disabled immediately.
> 
> Signed-off-by: Marc St-Amand <mstamand@ciena.com>
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Signed-off-by: Ben Peled <bpeled@marvell.com>
> ---
>  drivers/pci/controller/dwc/pcie-armada8k.c | 38 ++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
> index 13901f3..b2278b1 100644
> --- a/drivers/pci/controller/dwc/pcie-armada8k.c
> +++ b/drivers/pci/controller/dwc/pcie-armada8k.c
> @@ -54,6 +54,10 @@ struct armada8k_pcie {
>  #define PCIE_INT_C_ASSERT_MASK		BIT(11)
>  #define PCIE_INT_D_ASSERT_MASK		BIT(12)
>  
> +#define PCIE_GLOBAL_INT_CAUSE2_REG	(PCIE_VENDOR_REGS_OFFSET + 0x24)
> +#define PCIE_GLOBAL_INT_MASK2_REG	(PCIE_VENDOR_REGS_OFFSET + 0x28)
> +#define PCIE_INT2_PHY_RST_LINK_DOWN	BIT(1)
> +
>  #define PCIE_ARCACHE_TRC_REG		(PCIE_VENDOR_REGS_OFFSET + 0x50)
>  #define PCIE_AWCACHE_TRC_REG		(PCIE_VENDOR_REGS_OFFSET + 0x54)
>  #define PCIE_ARUSER_REG			(PCIE_VENDOR_REGS_OFFSET + 0x5C)
> @@ -193,6 +197,11 @@ static void armada8k_pcie_establish_link(struct armada8k_pcie *pcie)
>  	       PCIE_INT_C_ASSERT_MASK | PCIE_INT_D_ASSERT_MASK;
>  	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG, reg);
>  
> +	/* Also enable link down interrupts */
> +	reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG);
> +	reg |= PCIE_INT2_PHY_RST_LINK_DOWN;
> +	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG, reg);
> +
>  	if (!dw_pcie_link_up(pci)) {
>  		/* Configuration done. Start LTSSM */
>  		reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_CONTROL_REG);
> @@ -230,6 +239,35 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
>  	val = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_CAUSE1_REG);
>  	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_CAUSE1_REG, val);
>  
> +	val = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_CAUSE2_REG);
> +
> +	if (PCIE_INT2_PHY_RST_LINK_DOWN & val) {
> +		u32 ctrl_reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_CONTROL_REG);
> +		/*
> +		 * The link went down. Disable LTSSM immediately. This
> +		 * unlocks the root complex config registers. Downstream
> +		 * device accesses will return all-Fs
> +		 */

Hello! This looks like some issue with PCIe HW. Does it mean that if
LTSSM is not disabled then Root Complex stuck or crash during processing
config requests from CPU? Or what else happens?

Could you provide more details what is the exact issue described in this
comment? And is there any Marvell errata about this particular "disable
LTSSM immediately" issue?

> +		ctrl_reg &= ~(PCIE_APP_LTSSM_EN);
> +		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_CONTROL_REG, ctrl_reg);
> +		/*
> +		 * Mask link down interrupts. They can be re-enabled once
> +		 * the link is retrained.
> +		 */
> +		ctrl_reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG);
> +		ctrl_reg &= ~PCIE_INT2_PHY_RST_LINK_DOWN;
> +		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG, ctrl_reg);
> +		/*
> +		 * At this point a worker thread can be triggered to
> +		 * initiate a link retrain. If link retrains were
> +		 * possible, that is.
> +		 */
> +		dev_dbg(pci->dev, "%s: link went down\n", __func__);
> +	}
> +
> +	/* Now clear the second interrupt cause. */
> +	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_CAUSE2_REG, val);
> +
>  	return IRQ_HANDLED;
>  }
>  
> -- 
> 2.7.4
>