On Mon, Nov 11, 2024 at 10:42:41PM -0800, Jenishkumar Maheshbhai Patel wrote:
> When a PCI link down condition is detected, the link training state
> machine must be disabled immediately.
Why?
"Immediately" has no meaning here. Arbitrary delays are possible and
must not break anything.
> Signed-off-by: Jenishkumar Maheshbhai Patel <jpatel2@xxxxxxxxxxx>
> ---
> 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 b5c599ccaacf..07775539b321 100644
> --- a/drivers/pci/controller/dwc/pcie-armada8k.c
> +++ b/drivers/pci/controller/dwc/pcie-armada8k.c
> @@ -53,6 +53,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)
> @@ -204,6 +208,11 @@ static int armada8k_pcie_host_init(struct dw_pcie_rp *pp)
> 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);
> +
> return 0;
> }
>
> @@ -221,6 +230,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);
Add blank line.
> + /*
> + * The link went down. Disable LTSSM immediately. This
> + * unlocks the root complex config registers. Downstream
> + * device accesses will return all-Fs
> + */
> + ctrl_reg &= ~(PCIE_APP_LTSSM_EN);
> + dw_pcie_writel_dbi(pci, PCIE_GLOBAL_CONTROL_REG, ctrl_reg);
And here.
> + /*
> + * 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);
And here. Follow existing coding style in this file.
> + /*
> + * 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.25.1
>
