[RFC PATCH 1/4] PCI: pcie-rockchip: add bits for Target Link Speed in LCS_2

[RFC PATCH 1/4] PCI: pcie-rockchip: add bits for Target Link Speed in LCS_2

[Geraldo Nascimento]

Link Control and Status Register 2 is not present in current
pcie-rockchip.h definitions. Add it in preparation for
setting it before Gen2 retraining.

Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 drivers/pci/controller/pcie-rockchip.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index 14954f43e5e9..7a84899d3812 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -166,6 +166,9 @@
 #define   PCIE_RC_CONFIG_LINK_CAP_L0S		BIT(10)
 #define PCIE_RC_CONFIG_LCS		(PCIE_RC_CONFIG_BASE + 0xd0)
 #define PCIE_EP_CONFIG_LCS		(PCIE_EP_CONFIG_BASE + 0xd0)
+#define PCIE_RC_CONFIG_LCS_2		(PCIE_RC_CONFIG_BASE + 0xf0)
+#define   PCIE_RC_CONFIG_LCS_2_TLS_25	BIT(0)
+#define   PCIE_RC_CONFIG_LCS_2_TLS_50	BIT(1)
 #define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)
 #define PCIE_RC_CONFIG_THP_CAP		(PCIE_RC_CONFIG_BASE + 0x274)
 #define   PCIE_RC_CONFIG_THP_CAP_NEXT_MASK	GENMASK(31, 20)
-- 
2.49.0

Re: [RFC PATCH 1/4] PCI: pcie-rockchip: add bits for Target Link Speed in LCS_2

[Bjorn Helgaas]

On Sat, Jun 07, 2025 at 08:00:54AM -0300, Geraldo Nascimento wrote:
> Link Control and Status Register 2 is not present in current
> pcie-rockchip.h definitions. Add it in preparation for
> setting it before Gen2 retraining.
> 
> Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> ---
>  drivers/pci/controller/pcie-rockchip.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
> index 14954f43e5e9..7a84899d3812 100644
> --- a/drivers/pci/controller/pcie-rockchip.h
> +++ b/drivers/pci/controller/pcie-rockchip.h
> @@ -166,6 +166,9 @@
>  #define   PCIE_RC_CONFIG_LINK_CAP_L0S		BIT(10)
>  #define PCIE_RC_CONFIG_LCS		(PCIE_RC_CONFIG_BASE + 0xd0)
>  #define PCIE_EP_CONFIG_LCS		(PCIE_EP_CONFIG_BASE + 0xd0)
> +#define PCIE_RC_CONFIG_LCS_2		(PCIE_RC_CONFIG_BASE + 0xf0)
> +#define   PCIE_RC_CONFIG_LCS_2_TLS_25	BIT(0)
> +#define   PCIE_RC_CONFIG_LCS_2_TLS_50	BIT(1)

This stuff:

  #define PCIE_RC_CONFIG_DCR              (PCIE_RC_CONFIG_BASE + 0xc4)
  #define PCIE_RC_CONFIG_DCSR             (PCIE_RC_CONFIG_BASE + 0xc8)
  #define PCIE_RC_CONFIG_LINK_CAP         (PCIE_RC_CONFIG_BASE + 0xcc)
  #define PCIE_RC_CONFIG_LCS              (PCIE_RC_CONFIG_BASE + 0xd0)
  #define PCIE_RC_CONFIG_LCS_2            (PCIE_RC_CONFIG_BASE + 0xf0)

*Looks* like it might be duplicates of:

  #define PCI_EXP_DEVCAP          0x04    /* Device capabilities */
  #define PCI_EXP_DEVCTL          0x08    /* Device Control */
  #define PCI_EXP_LNKCAP          0x0c    /* Link Capabilities */
  #define PCI_EXP_LNKCTL          0x10    /* Link Control */
  #define PCI_EXP_LNKCTL2         0x30    /* Link Control 2 */

where the PCIe Capability is at (PCIE_RC_CONFIG_BASE + 0xc0).

If so, can you please rework these to use the existing PCI_EXP_*
definitions, including the fields inside?

>  #define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)
>  #define PCIE_RC_CONFIG_THP_CAP		(PCIE_RC_CONFIG_BASE + 0x274)
>  #define   PCIE_RC_CONFIG_THP_CAP_NEXT_MASK	GENMASK(31, 20)
> -- 
> 2.49.0
> 

Re: [RFC PATCH 1/4] PCI: pcie-rockchip: add bits for Target Link Speed in LCS_2

[Geraldo Nascimento]

On Tue, Jun 10, 2025 at 03:07:44PM -0500, Bjorn Helgaas wrote:
> This stuff:
> 
>   #define PCIE_RC_CONFIG_DCR              (PCIE_RC_CONFIG_BASE + 0xc4)
>   #define PCIE_RC_CONFIG_DCSR             (PCIE_RC_CONFIG_BASE + 0xc8)
>   #define PCIE_RC_CONFIG_LINK_CAP         (PCIE_RC_CONFIG_BASE + 0xcc)
>   #define PCIE_RC_CONFIG_LCS              (PCIE_RC_CONFIG_BASE + 0xd0)
>   #define PCIE_RC_CONFIG_LCS_2            (PCIE_RC_CONFIG_BASE + 0xf0)
> 
> *Looks* like it might be duplicates of:
> 
>   #define PCI_EXP_DEVCAP          0x04    /* Device capabilities */
>   #define PCI_EXP_DEVCTL          0x08    /* Device Control */
>   #define PCI_EXP_LNKCAP          0x0c    /* Link Capabilities */
>   #define PCI_EXP_LNKCTL          0x10    /* Link Control */
>   #define PCI_EXP_LNKCTL2         0x30    /* Link Control 2 */
> 
> where the PCIe Capability is at (PCIE_RC_CONFIG_BASE + 0xc0).
> 
> If so, can you please rework these to use the existing PCI_EXP_*
> definitions, including the fields inside?

Hi Bjorn,

I'll look into it, good catch indeed.

Thank you for your help!
Geraldo Nascimento

Re: [RFC PATCH 1/4] PCI: pcie-rockchip: add bits for Target Link Speed in LCS_2

[Geraldo Nascimento]

On Tue, Jun 10, 2025 at 03:07:44PM -0500, Bjorn Helgaas wrote:
> On Sat, Jun 07, 2025 at 08:00:54AM -0300, Geraldo Nascimento wrote:
> This stuff:
> 
>   #define PCIE_RC_CONFIG_DCR              (PCIE_RC_CONFIG_BASE + 0xc4)
>   #define PCIE_RC_CONFIG_DCSR             (PCIE_RC_CONFIG_BASE + 0xc8)
>   #define PCIE_RC_CONFIG_LINK_CAP         (PCIE_RC_CONFIG_BASE + 0xcc)
>   #define PCIE_RC_CONFIG_LCS              (PCIE_RC_CONFIG_BASE + 0xd0)
>   #define PCIE_RC_CONFIG_LCS_2            (PCIE_RC_CONFIG_BASE + 0xf0)
> 
> *Looks* like it might be duplicates of:
> 
>   #define PCI_EXP_DEVCAP          0x04    /* Device capabilities */
>   #define PCI_EXP_DEVCTL          0x08    /* Device Control */
>   #define PCI_EXP_LNKCAP          0x0c    /* Link Capabilities */

Hi again Bjorn,

Your message reminded me of something that may be important.

During my debugging I had the mild impression L0s capability is not
being cleared from Link Capabilities Register in the presence of
"aspm-no-l0s" DT property.

I can't confirm it right now but I might revisit this later on. From
what I've seen it can only be cleared from inside the port init
in pcie-rockchip.c and does nothing in present form.

Not a clear, confirmable report but something to watch out for...

Regards,
Geraldo Nascimento

Re: [RFC PATCH 1/4] PCI: pcie-rockchip: add bits for Target Link Speed in LCS_2

[Geraldo Nascimento]

On Wed, Jun 11, 2025 at 12:46:10AM -0300, Geraldo Nascimento wrote:
> Hi again Bjorn,
> 
> Your message reminded me of something that may be important.
> 
> During my debugging I had the mild impression L0s capability is not
> being cleared from Link Capabilities Register in the presence of
> "aspm-no-l0s" DT property.
> 
> I can't confirm it right now but I might revisit this later on. From
> what I've seen it can only be cleared from inside the port init
> in pcie-rockchip.c and does nothing in present form.
> 
> Not a clear, confirmable report but something to watch out for...

Not correct. ASPM bit for L0s is being properly cleared according
to my printk's.

Should have checked before creating noise. Sorry.
Geraldo Nascimento

> 
> Regards,
> Geraldo Nascimento