[PATCH v4 0/4] PCI: rockchip: 5.0 GT/s speed discouraged by Rockchip

[PATCH v4 0/4] PCI: rockchip: 5.0 GT/s speed discouraged by Rockchip

[Geraldo Nascimento]

Dragan Simic already had warned me of potential issues with 5.0 GT/s
speed operation in Rockchip PCIe. However, in recent interactions
with Shawn Lin from Rockchip it came to my attention there's grave
danger in the unknown errata regarding 5.0 GT/s operational speed
of their PCIe core.

Drop all code related to 5.0 GT/s operational speed from this driver.

Endpoint Mode driver was not tested. There's a bit of redundancy in the
commit messages but it's the best I could do to ensure people will
actually understand why perfectly working code is being dropped.

---

Changes in v4:
- Incorporate suggestion by Bjorn and refined by Dragan to drop the
  "catastrophic" code
- Link to v3: https://lore.kernel.org/linux-rockchip/cover.1772057799.git.geraldogabriel@gmail.com/T/

Changes in v3:
- Clarify warning message even though Rockchip won't disclose details
- Drop DT changes as they were applied as subset by Heiko
- Link to v2: https://lore.kernel.org/all/cover.1763415705.git.geraldogabriel@gmail.com/T/

Changes in v2:
- hard limit to 2.5 GT/s, not just warn
- add Reported-by: and Reviewed-by: Dragan Simic
- remove redundant declaration of max-link-speed from helios64 dts
- fix Link: of helios64 patch
- simplify RC mode comment
- Link to v1: https://lore.kernel.org/all/aRhR79u5BPtRRFw3@geday/T/


---

Geraldo Nascimento (4):
  PCI: rockchip: drop 2.5 GT/s defines
  PCI: rockchip: drive at 2.5 GT/s only and error out other speeds
  PCI: rockchip-host: do not attempt 5.0 GT/s retraining
  PCI: rockchip-ep: do not attempt 5.0 GT/s retraining

 drivers/pci/controller/pcie-rockchip-ep.c   | 13 -------------
 drivers/pci/controller/pcie-rockchip-host.c | 20 --------------------
 drivers/pci/controller/pcie-rockchip.c      | 13 +++++--------
 drivers/pci/controller/pcie-rockchip.h      |  3 ---
 4 files changed, 5 insertions(+), 44 deletions(-)

-- 
2.52.0

[PATCH v4 1/4] PCI: rockchip: drop 2.5 GT/s defines

[Geraldo Nascimento]

Drop the 2.5 GT/s Link Speed defines from Rockchip PCIe header
definitions. The reason is that Shawn Lin from Rockchip has
reiterated that there may be danger of "catastrophic failure"
in using their PCIe with 5.0 GT/s speeds.

While Rockchip has done so informally without issuing a proper
errata, and the particulars are thus unknown, this may cause
data loss or worse.

This change is corroborated by RK3399 official datasheet [1], which
states maximum link speed for this platform is 2.5 GT/s.

[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf

Fixes: 956cd99b35a8 ("PCI: rockchip: Separate common code from RC driver")
Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
Cc: stable@vger.kernel.org
Reported-by: Dragan Simic <dsimic@manjaro.org>
Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 drivers/pci/controller/pcie-rockchip.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index 3e82a69b9c00..b5da15601b58 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -42,7 +42,6 @@
 #define   PCIE_CLIENT_MODE_RC			HWORD_SET_BIT(0x0040)
 #define   PCIE_CLIENT_MODE_EP			HWORD_CLR_BIT(0x0040)
 #define   PCIE_CLIENT_GEN_SEL_1			HWORD_CLR_BIT(0x0080)
-#define   PCIE_CLIENT_GEN_SEL_2			HWORD_SET_BIT(0x0080)
 #define PCIE_CLIENT_LEGACY_INT_CTRL	(PCIE_CLIENT_BASE + 0x0c)
 #define   PCIE_CLIENT_INT_IN_ASSERT		HWORD_SET_BIT(0x0002)
 #define   PCIE_CLIENT_INT_IN_DEASSERT		HWORD_CLR_BIT(0x0002)
@@ -197,8 +196,6 @@
 	(((x) & PCIE_CORE_PL_CONF_LS_MASK) == PCIE_CORE_PL_CONF_LS_READY)
 #define PCIE_LINK_UP(x) \
 	(((x) & PCIE_CLIENT_LINK_STATUS_MASK) == PCIE_CLIENT_LINK_STATUS_UP)
-#define PCIE_LINK_IS_GEN2(x) \
-	(((x) & PCIE_CORE_PL_CONF_SPEED_MASK) == PCIE_CORE_PL_CONF_SPEED_5G)
 
 #define RC_REGION_0_ADDR_TRANS_H		0x00000000
 #define RC_REGION_0_ADDR_TRANS_L		0x00000000
-- 
2.52.0

Re: [PATCH v4 1/4] PCI: rockchip: drop 2.5 GT/s defines

[Charalampos Mitrodimas]

Geraldo Nascimento <geraldogabriel@gmail.com> writes:

> Drop the 2.5 GT/s Link Speed defines from Rockchip PCIe header
> definitions. The reason is that Shawn Lin from Rockchip has
> reiterated that there may be danger of "catastrophic failure"
> in using their PCIe with 5.0 GT/s speeds.
>
> While Rockchip has done so informally without issuing a proper
> errata, and the particulars are thus unknown, this may cause
> data loss or worse.
>
> This change is corroborated by RK3399 official datasheet [1], which
> states maximum link speed for this platform is 2.5 GT/s.
>
> [1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
>
> Fixes: 956cd99b35a8 ("PCI: rockchip: Separate common code from RC driver")
> Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> Cc: stable@vger.kernel.org
> Reported-by: Dragan Simic <dsimic@manjaro.org>
> Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> ---
>  drivers/pci/controller/pcie-rockchip.h | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
> index 3e82a69b9c00..b5da15601b58 100644
> --- a/drivers/pci/controller/pcie-rockchip.h
> +++ b/drivers/pci/controller/pcie-rockchip.h
> @@ -42,7 +42,6 @@
>  #define   PCIE_CLIENT_MODE_RC			HWORD_SET_BIT(0x0040)
>  #define   PCIE_CLIENT_MODE_EP			HWORD_CLR_BIT(0x0040)
>  #define   PCIE_CLIENT_GEN_SEL_1			HWORD_CLR_BIT(0x0080)
> -#define   PCIE_CLIENT_GEN_SEL_2			HWORD_SET_BIT(0x0080)

This commit alone won't compile right? PCIE_CLIENT_GEN_SEL_2 is still
referenced by rockchip_pcie_init_port():

         if (rockchip->link_gen == 2)
                 rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_2,
                                     PCIE_CLIENT_CONFIG);

This reference is removed by the next patch in the series but not sure
if we want to do it here instead.

>  #define PCIE_CLIENT_LEGACY_INT_CTRL	(PCIE_CLIENT_BASE + 0x0c)
>  #define   PCIE_CLIENT_INT_IN_ASSERT		HWORD_SET_BIT(0x0002)
>  #define   PCIE_CLIENT_INT_IN_DEASSERT		HWORD_CLR_BIT(0x0002)
> @@ -197,8 +196,6 @@
>  	(((x) & PCIE_CORE_PL_CONF_LS_MASK) == PCIE_CORE_PL_CONF_LS_READY)
>  #define PCIE_LINK_UP(x) \
>  	(((x) & PCIE_CLIENT_LINK_STATUS_MASK) == PCIE_CLIENT_LINK_STATUS_UP)
> -#define PCIE_LINK_IS_GEN2(x) \
> -	(((x) & PCIE_CORE_PL_CONF_SPEED_MASK) == PCIE_CORE_PL_CONF_SPEED_5G)

Same for this, references elsewhere tho.

>  
>  #define RC_REGION_0_ADDR_TRANS_H		0x00000000
>  #define RC_REGION_0_ADDR_TRANS_L		0x00000000

Re: [PATCH v4 1/4] PCI: rockchip: drop 2.5 GT/s defines

[Geraldo Nascimento]

Hi Charalampos,

On Fri, Feb 27, 2026 at 04:53:00PM +0000, Charalampos Mitrodimas wrote:
> Geraldo Nascimento <geraldogabriel@gmail.com> writes:
> 
> > Drop the 2.5 GT/s Link Speed defines from Rockchip PCIe header
> > definitions. The reason is that Shawn Lin from Rockchip has
> > reiterated that there may be danger of "catastrophic failure"
> > in using their PCIe with 5.0 GT/s speeds.
> >
> > While Rockchip has done so informally without issuing a proper
> > errata, and the particulars are thus unknown, this may cause
> > data loss or worse.
> >
> > This change is corroborated by RK3399 official datasheet [1], which
> > states maximum link speed for this platform is 2.5 GT/s.
> >
> > [1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
> >
> > Fixes: 956cd99b35a8 ("PCI: rockchip: Separate common code from RC driver")
> > Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> > Cc: stable@vger.kernel.org
> > Reported-by: Dragan Simic <dsimic@manjaro.org>
> > Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > ---
> >  drivers/pci/controller/pcie-rockchip.h | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
> > index 3e82a69b9c00..b5da15601b58 100644
> > --- a/drivers/pci/controller/pcie-rockchip.h
> > +++ b/drivers/pci/controller/pcie-rockchip.h
> > @@ -42,7 +42,6 @@
> >  #define   PCIE_CLIENT_MODE_RC			HWORD_SET_BIT(0x0040)
> >  #define   PCIE_CLIENT_MODE_EP			HWORD_CLR_BIT(0x0040)
> >  #define   PCIE_CLIENT_GEN_SEL_1			HWORD_CLR_BIT(0x0080)
> > -#define   PCIE_CLIENT_GEN_SEL_2			HWORD_SET_BIT(0x0080)
> 
> This commit alone won't compile right? PCIE_CLIENT_GEN_SEL_2 is still
> referenced by rockchip_pcie_init_port():

You are correct, thanks for catching it.

> 
>          if (rockchip->link_gen == 2)
>                  rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_2,
>                                      PCIE_CLIENT_CONFIG);
> 
> This reference is removed by the next patch in the series but not sure
> if we want to do it here instead.
> 
> >  #define PCIE_CLIENT_LEGACY_INT_CTRL	(PCIE_CLIENT_BASE + 0x0c)
> >  #define   PCIE_CLIENT_INT_IN_ASSERT		HWORD_SET_BIT(0x0002)
> >  #define   PCIE_CLIENT_INT_IN_DEASSERT		HWORD_CLR_BIT(0x0002)
> > @@ -197,8 +196,6 @@
> >  	(((x) & PCIE_CORE_PL_CONF_LS_MASK) == PCIE_CORE_PL_CONF_LS_READY)
> >  #define PCIE_LINK_UP(x) \
> >  	(((x) & PCIE_CLIENT_LINK_STATUS_MASK) == PCIE_CLIENT_LINK_STATUS_UP)
> > -#define PCIE_LINK_IS_GEN2(x) \
> > -	(((x) & PCIE_CORE_PL_CONF_SPEED_MASK) == PCIE_CORE_PL_CONF_SPEED_5G)
> 
> Same for this, references elsewhere tho.
> 

Thanks for pointing this out. I'll do better for v5.

Geraldo Nascimento

> >  
> >  #define RC_REGION_0_ADDR_TRANS_H		0x00000000
> >  #define RC_REGION_0_ADDR_TRANS_L		0x00000000

[PATCH v4 2/4] PCI: rockchip: drive at 2.5 GT/s only and error out other speeds

[Geraldo Nascimento]

Configure the core to be driven at 2.5 GT/s Link Speed and error
out on any other speed. The reason is that Shawn Lin from Rockchip
has reiterated that there may be danger of "catastrophic failure"
in using their PCIe with 5.0 GT/s speeds.

While Rockchip has done so informally without issuing a proper
errata, and the particulars are thus unknown, this may cause
data loss or worse.

This change is corroborated by RK3399 official datasheet [1], which
states maximum link speed for this platform is 2.5 GT/s.

[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf

Fixes: 956cd99b35a8 ("PCI: rockchip: Separate common code from RC driver")
Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
Cc: stable@vger.kernel.org
Reported-by: Dragan Simic <dsimic@manjaro.org>
Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 drivers/pci/controller/pcie-rockchip.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
index 0f88da378805..26fc350a66c1 100644
--- a/drivers/pci/controller/pcie-rockchip.c
+++ b/drivers/pci/controller/pcie-rockchip.c
@@ -66,8 +66,9 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
 	}
 
 	rockchip->link_gen = of_pci_get_max_link_speed(node);
-	if (rockchip->link_gen < 0 || rockchip->link_gen > 2)
-		rockchip->link_gen = 2;
+	if (rockchip->link_gen < 0 || rockchip->link_gen >= 2)
+		return dev_err_probe(dev, rockchip->link_gen,
+				     "Invalid Link Speed\n");
 
 	for (i = 0; i < ROCKCHIP_NUM_PM_RSTS; i++)
 		rockchip->pm_rsts[i].id = rockchip_pci_pm_rsts[i];
@@ -147,12 +148,8 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 		goto err_exit_phy;
 	}
 
-	if (rockchip->link_gen == 2)
-		rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_2,
-				    PCIE_CLIENT_CONFIG);
-	else
-		rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_1,
-				    PCIE_CLIENT_CONFIG);
+	rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_1,
+			    PCIE_CLIENT_CONFIG);
 
 	regs = PCIE_CLIENT_ARI_ENABLE |
 	       PCIE_CLIENT_CONF_LANE_NUM(rockchip->lanes);
-- 
2.52.0

Re: [PATCH v4 2/4] PCI: rockchip: drive at 2.5 GT/s only and error out other speeds

[Dragan Simic]

Hello Geraldo,

On Friday, February 27, 2026 06:36 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> Configure the core to be driven at 2.5 GT/s Link Speed and error
> out on any other speed. The reason is that Shawn Lin from Rockchip
> has reiterated that there may be danger of "catastrophic failure"
> in using their PCIe with 5.0 GT/s speeds.
> 
> While Rockchip has done so informally without issuing a proper
> errata, and the particulars are thus unknown, this may cause
> data loss or worse.
> 
> This change is corroborated by RK3399 official datasheet [1], which
> states maximum link speed for this platform is 2.5 GT/s.
> 
> [1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
> 
> Fixes: 956cd99b35a8 ("PCI: rockchip: Separate common code from RC driver")
> Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> Cc: stable@vger.kernel.org
> Reported-by: Dragan Simic <dsimic@manjaro.org>
> Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> ---
>  drivers/pci/controller/pcie-rockchip.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
> index 0f88da378805..26fc350a66c1 100644
> --- a/drivers/pci/controller/pcie-rockchip.c
> +++ b/drivers/pci/controller/pcie-rockchip.c
> @@ -66,8 +66,9 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
>  	}
>  
>  	rockchip->link_gen = of_pci_get_max_link_speed(node);
> -	if (rockchip->link_gen < 0 || rockchip->link_gen > 2)
> -		rockchip->link_gen = 2;
> +	if (rockchip->link_gen < 0 || rockchip->link_gen >= 2)
> +		return dev_err_probe(dev, rockchip->link_gen,
> +				     "Invalid Link Speed\n");

I'm quite surprised to see what happened here in the v4?  The changes
introduced in this diff block in the v3 were perfectly fine, i.e. we need
to correct any runtime occurrences of Gen2 speed setting in the rockchip_
pcie_parse_dt() function, together with emitting a warning that urges
the users and the board dtb maintainer to fix the board dtb.  I'll get
back to this below.

>  	for (i = 0; i < ROCKCHIP_NUM_PM_RSTS; i++)
>  		rockchip->pm_rsts[i].id = rockchip_pci_pm_rsts[i];
> @@ -147,12 +148,8 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  		goto err_exit_phy;
>  	}
>  
> -	if (rockchip->link_gen == 2)
> -		rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_2,
> -				    PCIE_CLIENT_CONFIG);
> -	else
> -		rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_1,
> -				    PCIE_CLIENT_CONFIG);
> +	rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_1,
> +			    PCIE_CLIENT_CONFIG);
>  
>  	regs = PCIE_CLIENT_ARI_ENABLE |
>  	       PCIE_CLIENT_CONF_LANE_NUM(rockchip->lanes);

At this point we need to check and bail out if no longer supported Gen2
speed setting is received, which also applies to the appropriate spot in
the PCIe endpoint driver.

To clarify, it's the responsibility of the rockchip_pcie_parse_dt()
function to validate and possibly correct the speed setting, because it
is what receives that setting as a value from the board dtb, while the
consumers of that validated speed setting should check it, to make sure
the speed is right because they no longer can handle requests for Gen2
speed, and simply bail out if the received speed isn't right, i.e. if
it differs from Gen1.

Re: [PATCH v4 2/4] PCI: rockchip: drive at 2.5 GT/s only and error out other speeds

[Geraldo Nascimento]

On Fri, Feb 27, 2026 at 06:33:34PM +0100, Dragan Simic wrote:
> Hello Geraldo,
>

Hi Dragan,

> On Friday, February 27, 2026 06:36 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > Configure the core to be driven at 2.5 GT/s Link Speed and error
> > out on any other speed. The reason is that Shawn Lin from Rockchip
> > has reiterated that there may be danger of "catastrophic failure"
> > in using their PCIe with 5.0 GT/s speeds.
> > 
> > While Rockchip has done so informally without issuing a proper
> > errata, and the particulars are thus unknown, this may cause
> > data loss or worse.
> > 
> > This change is corroborated by RK3399 official datasheet [1], which
> > states maximum link speed for this platform is 2.5 GT/s.
> > 
> > [1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
> > 
> > Fixes: 956cd99b35a8 ("PCI: rockchip: Separate common code from RC driver")
> > Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> > Cc: stable@vger.kernel.org
> > Reported-by: Dragan Simic <dsimic@manjaro.org>
> > Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > ---
> >  drivers/pci/controller/pcie-rockchip.c | 13 +++++--------
> >  1 file changed, 5 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
> > index 0f88da378805..26fc350a66c1 100644
> > --- a/drivers/pci/controller/pcie-rockchip.c
> > +++ b/drivers/pci/controller/pcie-rockchip.c
> > @@ -66,8 +66,9 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
> >  	}
> >  
> >  	rockchip->link_gen = of_pci_get_max_link_speed(node);
> > -	if (rockchip->link_gen < 0 || rockchip->link_gen > 2)
> > -		rockchip->link_gen = 2;
> > +	if (rockchip->link_gen < 0 || rockchip->link_gen >= 2)
> > +		return dev_err_probe(dev, rockchip->link_gen,
> > +				     "Invalid Link Speed\n");
> 
> I'm quite surprised to see what happened here in the v4?  The changes
> introduced in this diff block in the v3 were perfectly fine, i.e. we need
> to correct any runtime occurrences of Gen2 speed setting in the rockchip_
> pcie_parse_dt() function, together with emitting a warning that urges
> the users and the board dtb maintainer to fix the board dtb.  I'll get
> back to this below.

I see what you mean now. Sure, this could be incorporated for v5 without
a problem and is the more proper way to solve it.

> 
> >  	for (i = 0; i < ROCKCHIP_NUM_PM_RSTS; i++)
> >  		rockchip->pm_rsts[i].id = rockchip_pci_pm_rsts[i];
> > @@ -147,12 +148,8 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
> >  		goto err_exit_phy;
> >  	}
> >  
> > -	if (rockchip->link_gen == 2)
> > -		rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_2,
> > -				    PCIE_CLIENT_CONFIG);
> > -	else
> > -		rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_1,
> > -				    PCIE_CLIENT_CONFIG);
> > +	rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_1,
> > +			    PCIE_CLIENT_CONFIG);
> >  
> >  	regs = PCIE_CLIENT_ARI_ENABLE |
> >  	       PCIE_CLIENT_CONF_LANE_NUM(rockchip->lanes);
> 
> At this point we need to check and bail out if no longer supported Gen2
> speed setting is received, which also applies to the appropriate spot in
> the PCIe endpoint driver.

Right, it is a bit of a double-check I think but it can't hurt.

> 
> To clarify, it's the responsibility of the rockchip_pcie_parse_dt()
> function to validate and possibly correct the speed setting, because it
> is what receives that setting as a value from the board dtb, while the
> consumers of that validated speed setting should check it, to make sure
> the speed is right because they no longer can handle requests for Gen2
> speed, and simply bail out if the received speed isn't right, i.e. if
> it differs from Gen1.
> 

Like I said, it ends being a double-check, because we can't let probe
progress if link_gen ends up being different than 1. Either we force
2.5 GT/s with a fair warning or we error out on the probe already.

Thank you,
Geraldo Nascimento

Re: [PATCH v4 2/4] PCI: rockchip: drive at 2.5 GT/s only and error out other speeds

[Dragan Simic]

On Friday, February 27, 2026 23:47 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> On Fri, Feb 27, 2026 at 06:33:34PM +0100, Dragan Simic wrote:
> > On Friday, February 27, 2026 06:36 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > > Configure the core to be driven at 2.5 GT/s Link Speed and error
> > > out on any other speed. The reason is that Shawn Lin from Rockchip
> > > has reiterated that there may be danger of "catastrophic failure"
> > > in using their PCIe with 5.0 GT/s speeds.
> > > 
> > > While Rockchip has done so informally without issuing a proper
> > > errata, and the particulars are thus unknown, this may cause
> > > data loss or worse.
> > > 
> > > This change is corroborated by RK3399 official datasheet [1], which
> > > states maximum link speed for this platform is 2.5 GT/s.
> > > 
> > > [1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
> > > 
> > > Fixes: 956cd99b35a8 ("PCI: rockchip: Separate common code from RC driver")
> > > Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> > > Cc: stable@vger.kernel.org
> > > Reported-by: Dragan Simic <dsimic@manjaro.org>
> > > Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> > > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > > ---
> > >  drivers/pci/controller/pcie-rockchip.c | 13 +++++--------
> > >  1 file changed, 5 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
> > > index 0f88da378805..26fc350a66c1 100644
> > > --- a/drivers/pci/controller/pcie-rockchip.c
> > > +++ b/drivers/pci/controller/pcie-rockchip.c
> > > @@ -66,8 +66,9 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
> > >  	}
> > >  
> > >  	rockchip->link_gen = of_pci_get_max_link_speed(node);
> > > -	if (rockchip->link_gen < 0 || rockchip->link_gen > 2)
> > > -		rockchip->link_gen = 2;
> > > +	if (rockchip->link_gen < 0 || rockchip->link_gen >= 2)
> > > +		return dev_err_probe(dev, rockchip->link_gen,
> > > +				     "Invalid Link Speed\n");
> > 
> > I'm quite surprised to see what happened here in the v4?  The changes
> > introduced in this diff block in the v3 were perfectly fine, i.e. we need
> > to correct any runtime occurrences of Gen2 speed setting in the rockchip_
> > pcie_parse_dt() function, together with emitting a warning that urges
> > the users and the board dtb maintainer to fix the board dtb.  I'll get
> > back to this below.
> 
> I see what you mean now. Sure, this could be incorporated for v5 without
> a problem and is the more proper way to solve it.

Actually, we need to do it the way I suggested above, i.e. the v3 way,
because doing it like in the v4 diff chunk above could break PCIe on some
boards that currently have Gen2 configured in their dtb files, which may
get their kernels updated without updating their dtb files as well, for
whatever reason.  Basically, we need to avoid such breakage scenarios.

> > >  	for (i = 0; i < ROCKCHIP_NUM_PM_RSTS; i++)
> > >  		rockchip->pm_rsts[i].id = rockchip_pci_pm_rsts[i];
> > > @@ -147,12 +148,8 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
> > >  		goto err_exit_phy;
> > >  	}
> > >  
> > > -	if (rockchip->link_gen == 2)
> > > -		rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_2,
> > > -				    PCIE_CLIENT_CONFIG);
> > > -	else
> > > -		rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_1,
> > > -				    PCIE_CLIENT_CONFIG);
> > > +	rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_1,
> > > +			    PCIE_CLIENT_CONFIG);
> > >  
> > >  	regs = PCIE_CLIENT_ARI_ENABLE |
> > >  	       PCIE_CLIENT_CONF_LANE_NUM(rockchip->lanes);
> > 
> > At this point we need to check and bail out if no longer supported Gen2
> > speed setting is received, which also applies to the appropriate spot in
> > the PCIe endpoint driver.
> 
> Right, it is a bit of a double-check I think but it can't hurt.
> 
> > To clarify, it's the responsibility of the rockchip_pcie_parse_dt()
> > function to validate and possibly correct the speed setting, because it
> > is what receives that setting as a value from the board dtb, while the
> > consumers of that validated speed setting should check it, to make sure
> > the speed is right because they no longer can handle requests for Gen2
> > speed, and simply bail out if the received speed isn't right, i.e. if
> > it differs from Gen1.
> 
> Like I said, it ends being a double-check, because we can't let probe
> progress if link_gen ends up being different than 1. Either we force
> 2.5 GT/s with a fair warning or we error out on the probe already.

Ah, sorry, I somehow missed that both rockchip_pcie_host_init_port() and
rockchip_pcie_ep_link_training() are defined as static functions, so you're
right that checking for "link_gen == 1" in them again would be redundant.

[PATCH v4 3/4] PCI: rockchip-host: do not attempt 5.0 GT/s retraining

[Geraldo Nascimento]

Drop the 5.0 GT/s Link Speed retraining from Rockchip PCIe Root
Complex Mode Operation, so called host driver.

The reason is that Shawn Lin from Rockchip has reiterated that there
may be danger of "catastrophic failure" in using their PCIe with
5.0 GT/s speeds.

While Rockchip has done so informally without issuing a proper
errata, and the particulars are thus unknown, this may cause data
loss or worse.

This change is corroborated by RK3399 official datasheet [1], which
states maximum link speed for this platform is 2.5 GT/s.

[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf

Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
Cc: stable@vger.kernel.org
Reported-by: Dragan Simic <dsimic@manjaro.org>
Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 drivers/pci/controller/pcie-rockchip-host.c | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index ee1822ca01db..1374a2c92b56 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -328,26 +328,6 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
 		goto err_power_off_phy;
 	}
 
-	if (rockchip->link_gen == 2) {
-		/*
-		 * Enable retrain for gen2. This should be configured only after
-		 * gen1 finished.
-		 */
-		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
-		status &= ~PCI_EXP_LNKCTL2_TLS;
-		status |= PCI_EXP_LNKCTL2_TLS_5_0GT;
-		rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
-		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL);
-		status |= PCI_EXP_LNKCTL_RL;
-		rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL);
-
-		err = readl_poll_timeout(rockchip->apb_base + PCIE_CORE_CTRL,
-					 status, PCIE_LINK_IS_GEN2(status), 20,
-					 500 * USEC_PER_MSEC);
-		if (err)
-			dev_dbg(dev, "PCIe link training gen2 timeout, fall back to gen1!\n");
-	}
-
 	/* Check the final link width from negotiated lane counter from MGMT */
 	status = rockchip_pcie_read(rockchip, PCIE_CORE_CTRL);
 	status = 0x1 << ((status & PCIE_CORE_PL_CONF_LANE_MASK) >>
-- 
2.52.0

[PATCH v4 4/4] PCI: rockchip-ep: do not attempt 5.0 GT/s retraining

[Geraldo Nascimento]

Drop the 5.0 GT/s Link Speed retraining code block from Rockchip PCIe
header definitions. The reason is that Shawn Lin from Rockchip has
reiterated that there may be danger of "catastrophic failure" in
using their PCIe with 5.0 GT/s speeds.

While Rockchip has done so informally without issuing a proper
errata, and the particulars are thus unknown, this may cause data
loss or worse.

This change is corroborated by RK3399 official datasheet [1], which
states maximum link speed for this platform is 2.5 GT/s.

[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf

Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
Cc: stable@vger.kernel.org
Reported-by: Dragan Simic <dsimic@manjaro.org>
Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 drivers/pci/controller/pcie-rockchip-ep.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 799461335762..9ebc227a1ef8 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -553,19 +553,6 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work)
 	if (ret)
 		goto again;
 
-	/*
-	 * Check the current speed: if gen2 speed was requested and we are not
-	 * at gen2 speed yet, retrain again for gen2.
-	 */
-	val = rockchip_pcie_read(rockchip, PCIE_CORE_CTRL);
-	if (!PCIE_LINK_IS_GEN2(val) && rockchip->link_gen == 2) {
-		/* Enable retrain for gen2 */
-		rockchip_pcie_ep_retrain_link(rockchip);
-		readl_poll_timeout(rockchip->apb_base + PCIE_CORE_CTRL,
-				   val, PCIE_LINK_IS_GEN2(val), 50,
-				   LINK_TRAIN_TIMEOUT);
-	}
-
 	/* Check again that the link is up */
 	if (!rockchip_pcie_ep_link_up(rockchip))
 		goto again;
-- 
2.52.0

Re: [PATCH v4 4/4] PCI: rockchip-ep: do not attempt 5.0 GT/s retraining

[Charalampos Mitrodimas]

Geraldo Nascimento <geraldogabriel@gmail.com> writes:

> Drop the 5.0 GT/s Link Speed retraining code block from Rockchip PCIe
> header definitions. The reason is that Shawn Lin from Rockchip has
> reiterated that there may be danger of "catastrophic failure" in
> using their PCIe with 5.0 GT/s speeds.

Patch body says "header definitions" but the change is to the drivers'
source. Maybe you need to reword in a way that is clear as to what
happened?

Cheers!

>
> While Rockchip has done so informally without issuing a proper
> errata, and the particulars are thus unknown, this may cause data
> loss or worse.
>
> This change is corroborated by RK3399 official datasheet [1], which
> states maximum link speed for this platform is 2.5 GT/s.
>
> [1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
>
> Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> Cc: stable@vger.kernel.org
> Reported-by: Dragan Simic <dsimic@manjaro.org>
> Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> ---
>  drivers/pci/controller/pcie-rockchip-ep.c | 13 -------------
>  1 file changed, 13 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
> index 799461335762..9ebc227a1ef8 100644
> --- a/drivers/pci/controller/pcie-rockchip-ep.c
> +++ b/drivers/pci/controller/pcie-rockchip-ep.c
> @@ -553,19 +553,6 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work)
>  	if (ret)
>  		goto again;
>  
> -	/*
> -	 * Check the current speed: if gen2 speed was requested and we are not
> -	 * at gen2 speed yet, retrain again for gen2.
> -	 */
> -	val = rockchip_pcie_read(rockchip, PCIE_CORE_CTRL);
> -	if (!PCIE_LINK_IS_GEN2(val) && rockchip->link_gen == 2) {
> -		/* Enable retrain for gen2 */
> -		rockchip_pcie_ep_retrain_link(rockchip);
> -		readl_poll_timeout(rockchip->apb_base + PCIE_CORE_CTRL,
> -				   val, PCIE_LINK_IS_GEN2(val), 50,
> -				   LINK_TRAIN_TIMEOUT);
> -	}
> -
>  	/* Check again that the link is up */
>  	if (!rockchip_pcie_ep_link_up(rockchip))
>  		goto again;

Re: [PATCH v4 4/4] PCI: rockchip-ep: do not attempt 5.0 GT/s retraining

[Geraldo Nascimento]

On Fri, Feb 27, 2026 at 05:00:03PM +0000, Charalampos Mitrodimas wrote:
> Geraldo Nascimento <geraldogabriel@gmail.com> writes:
> 
> > Drop the 5.0 GT/s Link Speed retraining code block from Rockchip PCIe
> > header definitions. The reason is that Shawn Lin from Rockchip has
> > reiterated that there may be danger of "catastrophic failure" in
> > using their PCIe with 5.0 GT/s speeds.
> 
> Patch body says "header definitions" but the change is to the drivers'
> source. Maybe you need to reword in a way that is clear as to what
> happened?
> 
> Cheers!

Indeed, I was the victim of reckless copy pasting here.

Will fix for v5.

Thanks,
Geraldo Nascimento