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.
