* Re: [PATCH] PCI: dw-rockchip: Remove PCIE_L0S_ENTRY check from rockchip_pcie_link_up()
@ 2025-04-09 9:19 ` Niklas Cassel
0 siblings, 0 replies; 16+ messages in thread
From: Niklas Cassel @ 2025-04-09 9:19 UTC (permalink / raw)
To: Shawn Lin
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
linux-pci, linux-rockchip
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).
Kind regards,
Niklas
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] PCI: dw-rockchip: Remove PCIE_L0S_ENTRY check from rockchip_pcie_link_up()
2025-04-09 9:19 ` Niklas Cassel
@ 2025-04-09 9:41 ` Shawn Lin
-1 siblings, 0 replies; 16+ messages in thread
From: Shawn Lin @ 2025-04-09 9:41 UTC (permalink / raw)
To: Niklas Cassel
Cc: shawn.lin, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, linux-pci, linux-rockchip
在 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
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] PCI: dw-rockchip: Remove PCIE_L0S_ENTRY check from rockchip_pcie_link_up()
@ 2025-04-09 9:41 ` Shawn Lin
0 siblings, 0 replies; 16+ messages in thread
From: Shawn Lin @ 2025-04-09 9:41 UTC (permalink / raw)
To: Niklas Cassel
Cc: shawn.lin, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, linux-pci, linux-rockchip
在 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
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] PCI: dw-rockchip: Remove PCIE_L0S_ENTRY check from rockchip_pcie_link_up()
2025-04-09 9:19 ` Niklas Cassel
@ 2025-04-13 14:24 ` Manivannan Sadhasivam
-1 siblings, 0 replies; 16+ messages in thread
From: Manivannan Sadhasivam @ 2025-04-13 14:24 UTC (permalink / raw)
To: Niklas Cassel
Cc: Shawn Lin, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, linux-pci, linux-rockchip
On Wed, Apr 09, 2025 at 11:19:03AM +0200, Niklas Cassel wrote:
> 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!
>
Right. But I'm also wondering if we should use the 'Data Link Layer Link Active'
bit in PCI Express Capability for checking link up. Qcom driver has been using
it from the start and there are no reported issues. We could add this as the
first fallback if the link_up callback is not provided.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] PCI: dw-rockchip: Remove PCIE_L0S_ENTRY check from rockchip_pcie_link_up()
@ 2025-04-13 14:24 ` Manivannan Sadhasivam
0 siblings, 0 replies; 16+ messages in thread
From: Manivannan Sadhasivam @ 2025-04-13 14:24 UTC (permalink / raw)
To: Niklas Cassel
Cc: Shawn Lin, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, linux-pci, linux-rockchip
On Wed, Apr 09, 2025 at 11:19:03AM +0200, Niklas Cassel wrote:
> 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!
>
Right. But I'm also wondering if we should use the 'Data Link Layer Link Active'
bit in PCI Express Capability for checking link up. Qcom driver has been using
it from the start and there are no reported issues. We could add this as the
first fallback if the link_up callback is not provided.
- Mani
--
மணிவண்ணன் சதாசிவம்
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] PCI: dw-rockchip: Remove PCIE_L0S_ENTRY check from rockchip_pcie_link_up()
2025-04-13 14:24 ` Manivannan Sadhasivam
@ 2025-04-14 10:06 ` Niklas Cassel
-1 siblings, 0 replies; 16+ messages in thread
From: Niklas Cassel @ 2025-04-14 10:06 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Shawn Lin, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, linux-pci, linux-rockchip
Hello Mani,
On Sun, Apr 13, 2025 at 07:54:28PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Apr 09, 2025 at 11:19:03AM +0200, Niklas Cassel wrote:
> >
> > 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!
> >
>
> Right. But I'm also wondering if we should use the 'Data Link Layer Link Active'
> bit in PCI Express Capability for checking link up. Qcom driver has been using
> it from the start and there are no reported issues. We could add this as the
> first fallback if the link_up callback is not provided.
Sounds like a good idea, but from looking at:
7.5.3.6 Link Capabilities Register (Offset 0Ch)
"
Data Link Layer Link Active Reporting Capable - For a Downstream Port,
this bit must be hardwired to 1b if the component supports the optional
capability of reporting the DL_Active state of the Data Link Control and
Management State Machine. For a hot-plug capable Downstream Port (as
indicated by the Hot-Plug Capable bit of the Slot Capabilities Register)
or a Downstream Port that supports Link speeds greater than 5.0 GT/s,
this bit must be hardwired to 1b.
For Upstream Ports and components that do not support this optional
capability, this bit must be hardwired to 0b.
"
It sounds like the the 'Data Link Layer Link Active' bit is optional,
or at least optional for Gen1 and Gen2.
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] PCI: dw-rockchip: Remove PCIE_L0S_ENTRY check from rockchip_pcie_link_up()
@ 2025-04-14 10:06 ` Niklas Cassel
0 siblings, 0 replies; 16+ messages in thread
From: Niklas Cassel @ 2025-04-14 10:06 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Shawn Lin, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, linux-pci, linux-rockchip
Hello Mani,
On Sun, Apr 13, 2025 at 07:54:28PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Apr 09, 2025 at 11:19:03AM +0200, Niklas Cassel wrote:
> >
> > 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!
> >
>
> Right. But I'm also wondering if we should use the 'Data Link Layer Link Active'
> bit in PCI Express Capability for checking link up. Qcom driver has been using
> it from the start and there are no reported issues. We could add this as the
> first fallback if the link_up callback is not provided.
Sounds like a good idea, but from looking at:
7.5.3.6 Link Capabilities Register (Offset 0Ch)
"
Data Link Layer Link Active Reporting Capable - For a Downstream Port,
this bit must be hardwired to 1b if the component supports the optional
capability of reporting the DL_Active state of the Data Link Control and
Management State Machine. For a hot-plug capable Downstream Port (as
indicated by the Hot-Plug Capable bit of the Slot Capabilities Register)
or a Downstream Port that supports Link speeds greater than 5.0 GT/s,
this bit must be hardwired to 1b.
For Upstream Ports and components that do not support this optional
capability, this bit must be hardwired to 0b.
"
It sounds like the the 'Data Link Layer Link Active' bit is optional,
or at least optional for Gen1 and Gen2.
Kind regards,
Niklas
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] PCI: dw-rockchip: Remove PCIE_L0S_ENTRY check from rockchip_pcie_link_up()
2025-04-14 10:06 ` Niklas Cassel
@ 2025-04-14 11:15 ` Manivannan Sadhasivam
-1 siblings, 0 replies; 16+ messages in thread
From: Manivannan Sadhasivam @ 2025-04-14 11:15 UTC (permalink / raw)
To: Niklas Cassel
Cc: Shawn Lin, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, linux-pci, linux-rockchip
On Mon, Apr 14, 2025 at 12:06:09PM +0200, Niklas Cassel wrote:
> Hello Mani,
>
> On Sun, Apr 13, 2025 at 07:54:28PM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Apr 09, 2025 at 11:19:03AM +0200, Niklas Cassel wrote:
> > >
> > > 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!
> > >
> >
> > Right. But I'm also wondering if we should use the 'Data Link Layer Link Active'
> > bit in PCI Express Capability for checking link up. Qcom driver has been using
> > it from the start and there are no reported issues. We could add this as the
> > first fallback if the link_up callback is not provided.
>
> Sounds like a good idea, but from looking at:
> 7.5.3.6 Link Capabilities Register (Offset 0Ch)
>
> "
> Data Link Layer Link Active Reporting Capable - For a Downstream Port,
> this bit must be hardwired to 1b if the component supports the optional
> capability of reporting the DL_Active state of the Data Link Control and
> Management State Machine. For a hot-plug capable Downstream Port (as
> indicated by the Hot-Plug Capable bit of the Slot Capabilities Register)
> or a Downstream Port that supports Link speeds greater than 5.0 GT/s,
> this bit must be hardwired to 1b.
>
> For Upstream Ports and components that do not support this optional
> capability, this bit must be hardwired to 0b.
> "
>
> It sounds like the the 'Data Link Layer Link Active' bit is optional,
> or at least optional for Gen1 and Gen2.
Yeah, we should avoid relying on it. Thanks for digging through the spec :)
Really appreciated!
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] PCI: dw-rockchip: Remove PCIE_L0S_ENTRY check from rockchip_pcie_link_up()
@ 2025-04-14 11:15 ` Manivannan Sadhasivam
0 siblings, 0 replies; 16+ messages in thread
From: Manivannan Sadhasivam @ 2025-04-14 11:15 UTC (permalink / raw)
To: Niklas Cassel
Cc: Shawn Lin, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, linux-pci, linux-rockchip
On Mon, Apr 14, 2025 at 12:06:09PM +0200, Niklas Cassel wrote:
> Hello Mani,
>
> On Sun, Apr 13, 2025 at 07:54:28PM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Apr 09, 2025 at 11:19:03AM +0200, Niklas Cassel wrote:
> > >
> > > 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!
> > >
> >
> > Right. But I'm also wondering if we should use the 'Data Link Layer Link Active'
> > bit in PCI Express Capability for checking link up. Qcom driver has been using
> > it from the start and there are no reported issues. We could add this as the
> > first fallback if the link_up callback is not provided.
>
> Sounds like a good idea, but from looking at:
> 7.5.3.6 Link Capabilities Register (Offset 0Ch)
>
> "
> Data Link Layer Link Active Reporting Capable - For a Downstream Port,
> this bit must be hardwired to 1b if the component supports the optional
> capability of reporting the DL_Active state of the Data Link Control and
> Management State Machine. For a hot-plug capable Downstream Port (as
> indicated by the Hot-Plug Capable bit of the Slot Capabilities Register)
> or a Downstream Port that supports Link speeds greater than 5.0 GT/s,
> this bit must be hardwired to 1b.
>
> For Upstream Ports and components that do not support this optional
> capability, this bit must be hardwired to 0b.
> "
>
> It sounds like the the 'Data Link Layer Link Active' bit is optional,
> or at least optional for Gen1 and Gen2.
Yeah, we should avoid relying on it. Thanks for digging through the spec :)
Really appreciated!
- Mani
--
மணிவண்ணன் சதாசிவம்
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ messages in thread