Re: [PATCH v2] PCI: dw-rockchip: Delay link training after hot reset in EP mode

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Jun 18, 2025 at 04:40:11PM +0200, Niklas Cassel wrote:
> On Wed, Jun 18, 2025 at 04:23:19PM +0200, Niklas Cassel wrote:
> > On Tue, Jun 17, 2025 at 05:05:23PM -0500, Bjorn Helgaas wrote:
> > > On Tue, Jun 17, 2025 at 05:01:16PM -0500, Bjorn Helgaas wrote:
> > > > On Fri, Jun 13, 2025 at 12:19:09PM +0200, Niklas Cassel wrote:
> > > 
> > > Oh, and this sets PCIE_LTSSM_ENABLE_ENHANCE | PCIE_LTSSM_APP_DLY2_EN
> > > once at probe-time, but what about after a link-down/link-up cycle?
> > > 
> > > Don't we need to set PCIE_LTSSM_ENABLE_ENHANCE |
> > > PCIE_LTSSM_APP_DLY2_EN again when the link comes up so we don't have
> > > the same race when the link goes down again?
> > 
> > Nope, we don't.
> > 
> > To verify I used this patch:
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > index be239254aacd..e79add5412b8 100644
> > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > @@ -506,6 +506,8 @@ static irqreturn_t rockchip_pcie_ep_sys_irq_thread(int irq, void *arg)
> >         if (reg & PCIE_LINK_REQ_RST_NOT_INT) {
> >                 dev_dbg(dev, "hot reset or link-down reset\n");
> >                 dw_pcie_ep_linkdown(&pci->ep);
> > +               pr_info("PCIE_CLIENT_HOT_RESET_CTRL after reset: %#x\n",
> > +                       rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_HOT_RESET_CTRL));
> >                 /* Stop delaying link training. */
> >                 val = HIWORD_UPDATE_BIT(PCIE_LTSSM_APP_DLY2_DONE);
> >                 rockchip_pcie_writel_apb(rockchip, val,
> > 
> > 
> > 
> > 
> > [   85.979567] rockchip-dw-pcie a40000000.pcie-ep: hot reset or link-down reset
> > [   85.980210] PCIE_CLIENT_HOT_RESET_CTRL after reset: 0x12
> > [   93.720413] rockchip-dw-pcie a40000000.pcie-ep: hot reset or link-down reset
> > [   93.721074] PCIE_CLIENT_HOT_RESET_CTRL after reset: 0x12
> > 
> > 0x12 == bit 1 and bit 4 are set.
> > 
> > bit 1: app_dly2_en
> > bit 4: app_ltssm_enable_enhance
> 
> Oh, and just to verify that the hardware does not clear the app_dly2_en bit
> when we write the app_dly2_done bit, I ran the same test, but with the
> prints in rockchip_pcie_ep_sys_irq_thread(), just after calling
> dw_pcie_ep_linkup(&pci->ep); and got the same result:
> 
> [   57.176862] rockchip-dw-pcie a40000000.pcie-ep: link up
> [   57.177338] PCIE_CLIENT_HOT_RESET_CTRL after linkup: 0x12
> [   72.448052] rockchip-dw-pcie a40000000.pcie-ep: link up
> [   72.448527] PCIE_CLIENT_HOT_RESET_CTRL after linkup: 0x12

Thanks, I had missed the difference between PCIE_LTSSM_APP_DLY2_EN and
PCIE_LTSSM_APP_DLY2_DONE.