在 2025/04/09 星期三 17:19, Niklas Cassel 写道:
Hello Shawn,
On Wed, Apr 09, 2025 at 05:09:38PM +0800, Shawn Lin wrote:
在 2025/04/09 星期三 16:30, Niklas Cassel 写道:
On Wed, Apr 09, 2025 at 02:40:33PM +0800, Shawn Lin wrote:
Is there any advantage of using a rockchip specific way to read link up,
rather than just reading link up via the DWC PCIE_PORT_DEBUG1 register?
This is a very good question which we tried but made real products
suffer from it for a long time, and finally we found the reason and
discarded it.
Quoted from DWC databook, section 8.2.3 AXI Bridge Initialization,
Clocking and Reset:
"In RC Mode, your AXI application must not generate any MEM or I/O
requests, until the host software has enabled the Memory Space Enable
(MSE), and IO Space Enable (ISE) bits respectively. Your RC application
should not generate CFG requests until it has confirmed that the link is
up by sampling the smlh_link_up and rdlh_link_up outputs."
Quoted from DWC databook, section 5.50 SII: Debug Signals
"[36]: smlh_link_up: LTSSM reports PHY link up or LTSSM is in
Loopback.Active for Loopback Master" which refers to
PCIE_PORT_DEBUG1_LINK_UP per code.
The timing in dwc core is drving smlh_link_up->L0->rdlh_link_up->FC
init(a fixed delay) from IC simulation when linking up.
The dw_pcie_link_up() wasn't reliably work as expected by massive test.
The problem is clear from our ASIC view, that cxpl_debug_info from DWC
core is missing rdlh_link_up. cxpl_debug_info[32:63] is indentical to
PCIE_PORT_DEBUG1, So reading PCIE_PORT_DEBUG1 and check
smlh_link_up isn't enough.
The problem was introduced by commit 1 and fixed by commit 2 but not to
the end. And finally commit 3 rename the register but not fix anything.
It was broken from the first time. Any dwc controllers should not be use
the buggy default method to check link up state from our view.
So this's the whole story for it, which may help you understand the
indeed problem and why we reinvent rockchip_pcie_link_up() here.
[1]. commit dac29e6c5460 ("PCI: designware: Add default link up check if
sub-driver doesn't override")
[2]. commit 01c076732e82 ("PCI: designware: Check LTSSM training bit
before deciding link is up")
[3]. commit 60ef4b072ba0 ("PCI: dwc: imx6: Share PHY debug register
definitions")
Thank you for the detailed answer.
It seems like we should really add a warning and a comment in
dw_pcie_link_up(), so that others don't get hit by this hard to debug issue!
(Especially since dw_pcie_link_up() was added by someone with a @synopsys.com
email).
Yep, will do it with another patch.
Kind regards,
Niklas
