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
So I think this patch is good as is.
(Except for the citation style in the commit message which you didn't like :))
Kind regards,
Niklas
