1. Patchew
  2. linux
  3. [RFC PATCH v2 0/4] Quality Improvements for Rockchip-IP PCIe
  4. View patch

[RFC PATCH v2 1/4] PCI: pcie-rockchip: add Link Control and Status Register 2

Geraldo Nascimento posted 4 patches 8 months ago
There is a newer version of this series
[RFC PATCH v2 1/4] PCI: pcie-rockchip: add Link Control and Status Register 2
Posted by Geraldo Nascimento 8 months ago 
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.

While at it, also reference other registers from offset at
Capabilities Register through standard PCI definitions. Only
RC registers have been touched, although in principle there's
no functional change.

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

diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index 5864a20323f2..90d98aa8830e 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -155,17 +155,19 @@
 #define PCIE_EP_CONFIG_DID_VID		(PCIE_EP_CONFIG_BASE + 0x00)
 #define PCIE_EP_CONFIG_LCS		(PCIE_EP_CONFIG_BASE + 0xd0)
 #define PCIE_RC_CONFIG_RID_CCR		(PCIE_RC_CONFIG_BASE + 0x08)
-#define PCIE_RC_CONFIG_DCR		(PCIE_RC_CONFIG_BASE + 0xc4)
+#define PCIE_RC_CONFIG_CR		(PCIE_RC_CONFIG_BASE + 0xc0)
+#define PCIE_RC_CONFIG_DCR		(PCIE_RC_CONFIG_CR + PCI_EXP_DEVCAP)
 #define   PCIE_RC_CONFIG_DCR_CSPL_SHIFT		18
 #define   PCIE_RC_CONFIG_DCR_CSPL_LIMIT		0xff
 #define   PCIE_RC_CONFIG_DCR_CPLS_SHIFT		26
-#define PCIE_RC_CONFIG_DCSR		(PCIE_RC_CONFIG_BASE + 0xc8)
+#define PCIE_RC_CONFIG_DCSR		(PCIE_RC_CONFIG_CR + PCI_EXP_DEVCTL)
 #define   PCIE_RC_CONFIG_DCSR_MPS_MASK		GENMASK(7, 5)
 #define   PCIE_RC_CONFIG_DCSR_MPS_256		(0x1 << 5)
-#define PCIE_RC_CONFIG_LINK_CAP		(PCIE_RC_CONFIG_BASE + 0xcc)
+#define PCIE_RC_CONFIG_LINK_CAP		(PCIE_RC_CONFIG_CR + PCI_EXP_LNKCAP)
 #define   PCIE_RC_CONFIG_LINK_CAP_L0S		BIT(10)
-#define PCIE_RC_CONFIG_LCS		(PCIE_RC_CONFIG_BASE + 0xd0)
+#define PCIE_RC_CONFIG_LCS		(PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL)
 #define PCIE_EP_CONFIG_LCS		(PCIE_EP_CONFIG_BASE + 0xd0)
+#define PCIE_RC_CONFIG_LCS_2		(PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2)
 #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 v2 1/4] PCI: pcie-rockchip: add Link Control and Status Register 2
Posted by Bjorn Helgaas 8 months ago 
On Tue, Jun 10, 2025 at 06:19:49PM -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.
> 
> While at it, also reference other registers from offset at
> Capabilities Register through standard PCI definitions. Only
> RC registers have been touched, although in principle there's
> no functional change.
> 
> Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> ---
>  drivers/pci/controller/pcie-rockchip.h | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
> index 5864a20323f2..90d98aa8830e 100644
> --- a/drivers/pci/controller/pcie-rockchip.h
> +++ b/drivers/pci/controller/pcie-rockchip.h
> @@ -155,17 +155,19 @@
>  #define PCIE_EP_CONFIG_DID_VID		(PCIE_EP_CONFIG_BASE + 0x00)
>  #define PCIE_EP_CONFIG_LCS		(PCIE_EP_CONFIG_BASE + 0xd0)
>  #define PCIE_RC_CONFIG_RID_CCR		(PCIE_RC_CONFIG_BASE + 0x08)
> -#define PCIE_RC_CONFIG_DCR		(PCIE_RC_CONFIG_BASE + 0xc4)
> +#define PCIE_RC_CONFIG_CR		(PCIE_RC_CONFIG_BASE + 0xc0)
> +#define PCIE_RC_CONFIG_DCR		(PCIE_RC_CONFIG_CR + PCI_EXP_DEVCAP)

I would really like to see PCI_EXP_DEVCAP referenced in the source
where we currently use PCIE_RC_CONFIG_DCR.  That way, cscope/tags/grep
will find the actual uses of PCI_EXP_DEVCAP, not just this #define of
PCIE_RC_CONFIG_DCR.

Something like this:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/pci-mvebu.c?id=v6.15#n265

>  #define   PCIE_RC_CONFIG_DCR_CSPL_SHIFT		18
>  #define   PCIE_RC_CONFIG_DCR_CSPL_LIMIT		0xff
>  #define   PCIE_RC_CONFIG_DCR_CPLS_SHIFT		26

Also use PCI_EXP_DEVCAP_PWR_VAL and PCI_EXP_DEVCAP_PWR_SCL here if
possible.  And FIELD_GET()/FIELD_PREP(), which avoid the need to
define _SHIFT values.

I would do a pure conversion patch of the existing #defines.  Then I
suspect you wouldn't need a patch to add the Link 2 registers at all
because you could just use the #defines from pci_regs.h.

> -#define PCIE_RC_CONFIG_DCSR		(PCIE_RC_CONFIG_BASE + 0xc8)
> +#define PCIE_RC_CONFIG_DCSR		(PCIE_RC_CONFIG_CR + PCI_EXP_DEVCTL)
>  #define   PCIE_RC_CONFIG_DCSR_MPS_MASK		GENMASK(7, 5)
>  #define   PCIE_RC_CONFIG_DCSR_MPS_256		(0x1 << 5)
> -#define PCIE_RC_CONFIG_LINK_CAP		(PCIE_RC_CONFIG_BASE + 0xcc)
> +#define PCIE_RC_CONFIG_LINK_CAP		(PCIE_RC_CONFIG_CR + PCI_EXP_LNKCAP)
>  #define   PCIE_RC_CONFIG_LINK_CAP_L0S		BIT(10)
> -#define PCIE_RC_CONFIG_LCS		(PCIE_RC_CONFIG_BASE + 0xd0)
> +#define PCIE_RC_CONFIG_LCS		(PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL)
>  #define PCIE_EP_CONFIG_LCS		(PCIE_EP_CONFIG_BASE + 0xd0)
> +#define PCIE_RC_CONFIG_LCS_2		(PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2)
>  #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 v2 1/4] PCI: pcie-rockchip: add Link Control and Status Register 2
Posted by Geraldo Nascimento 8 months ago 
On Wed, Jun 11, 2025 at 02:42:59PM -0500, Bjorn Helgaas wrote:
> I would do a pure conversion patch of the existing #defines.  Then I
> suspect you wouldn't need a patch to add the Link 2 registers at all
> because you could just use the #defines from pci_regs.h.

Hi Bjorn,

I've hit roadblock, maybe you can help?

PCIE_RC_CONFIG_DCR_CSPL_LIMIT is defined as 0xff...

I'd like to kill that define too, since it will be
orphaned.

But hardcoding 0xff seems like illegible solution.

Perhaps there is another standard define that
maps to 0xff that I can use? Anyone comes
to your mind?

Thanks!
Geraldo Nascimento
Re: [RFC PATCH v2 1/4] PCI: pcie-rockchip: add Link Control and Status Register 2
Posted by Bjorn Helgaas 8 months ago 
On Thu, Jun 12, 2025 at 05:49:57PM -0300, Geraldo Nascimento wrote:
> On Wed, Jun 11, 2025 at 02:42:59PM -0500, Bjorn Helgaas wrote:
> > I would do a pure conversion patch of the existing #defines.  Then I
> > suspect you wouldn't need a patch to add the Link 2 registers at all
> > because you could just use the #defines from pci_regs.h.
> 
> Hi Bjorn,
> 
> I've hit roadblock, maybe you can help?
> 
> PCIE_RC_CONFIG_DCR_CSPL_LIMIT is defined as 0xff...
> 
> I'd like to kill that define too, since it will be
> orphaned.
> 
> But hardcoding 0xff seems like illegible solution.
> 
> Perhaps there is another standard define that
> maps to 0xff that I can use? Anyone comes
> to your mind?

Maybe FIELD_MAX(PCI_EXP_DEVCAP_PWR_VAL)?
Re: [RFC PATCH v2 1/4] PCI: pcie-rockchip: add Link Control and Status Register 2
Posted by Geraldo Nascimento 8 months ago 
On Wed, Jun 11, 2025 at 02:42:59PM -0500, Bjorn Helgaas wrote:
> On Tue, Jun 10, 2025 at 06:19:49PM -0300, Geraldo Nascimento wrote:
> > +#define PCIE_RC_CONFIG_DCR		(PCIE_RC_CONFIG_CR + PCI_EXP_DEVCAP)
> 
> I would really like to see PCI_EXP_DEVCAP referenced in the source
> where we currently use PCIE_RC_CONFIG_DCR.  That way, cscope/tags/grep
> will find the actual uses of PCI_EXP_DEVCAP, not just this #define of
> PCIE_RC_CONFIG_DCR.
> 
> Something like this:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/pci-mvebu.c?id=v6.15#n265
>

Hi Bjorn,

Yes, thank you for the code snippet, it makes things more clear.

> >  #define   PCIE_RC_CONFIG_DCR_CSPL_SHIFT		18
> >  #define   PCIE_RC_CONFIG_DCR_CSPL_LIMIT		0xff
> >  #define   PCIE_RC_CONFIG_DCR_CPLS_SHIFT		26
> 
> Also use PCI_EXP_DEVCAP_PWR_VAL and PCI_EXP_DEVCAP_PWR_SCL here if
> possible.  And FIELD_GET()/FIELD_PREP(), which avoid the need to
> define _SHIFT values.
> 
> I would do a pure conversion patch of the existing #defines.  Then I
> suspect you wouldn't need a patch to add the Link 2 registers at all
> because you could just use the #defines from pci_regs.h.
>

Got it!

Thanks!
Geraldo Nascimento

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.