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

[Niklas Cassel <cassel@kernel.org>]

On Wed, Jun 18, 2025 at 02:59:59PM -0500, Bjorn Helgaas wrote:
> > 
> > Well, because currently, we do NOT delay link training, and everything
> > works as expected.
> > 
> > Most likely we are just lucky, because dw_pcie_ep_linkdown() calls
> > dw_pcie_ep_init_non_sticky_registers(), which is quite a short function.
> 
> I'm just making the point that IIUC there's a race between link
> training and any DBI accesses done by
> dw_pcie_ep_init_non_sticky_registers() and potentially EPF callbacks,
> and the time those paths take is immaterial.
> 
> If this is indeed a race and this patch is the fix, I think it's
> misleading to describe it as "this path might take a long time and
> lose the race."  We have to assume arbitrary delays can be added to
> either path, so we can never rely on a path being "fast enough" to
> avoid the race.

I agree 100%.

When writing the commit message, we simply wanted to be transparent that we
have not observed the problem that this fix (theoretical fix?) is solving.

However, from reading the TRM (trust me, a lot of hours...), we are certain
that this feature DLY2_EN + DLY2_DONE was implemented such that there would
not be a race between link training and reinitializing registers via the DBI.


> 
> Is the following basically what we're doing?
> 
>   Set PCIE_LTSSM_APP_DLY2_EN so the controller never automatically
>   trains the link after a link-down interrupt.  That way any DBI
>   updates done in the dw_pcie_ep_linkdown() path will happen while the
>   link is still down.  Then allow link training by setting
>   PCIE_LTSSM_APP_DLY2_DONE.

Yes.

s/link-down interrupt/link-down or hot reset interrupt/

When Hot Reset or Link-down reset occurs, controller will assert
smlh_req_rst_not as an early warning. This warning is an interrupt bit in
Client Register group(link_req_rst_not_int).

(It is the same IRQ, so we can't tell the difference, at least not from
only looking at the IRQ status.)


> 
> We don't set PCIE_LTSSM_APP_DLY2_DONE anywhere in the initial probe
> path.  Obviously the link must train in that case, so I guess
> PCIE_LTSSM_APP_DLY2_EN only applies to the case of link state
> transition from link-up to link-down?

Yes.

The register description for dly2_en:
Set "1" to enable delaying the link training after Hot Reset.

The register description for dly2_done:
Set "1" to end the delaying of the link training after Hot Reset.


Kind regards,
Niklas