[PATCH v3 0/2] PCI: rockchip: 5.0 GT/s speed may be dangerous

[PATCH v3 0/2] PCI: rockchip: 5.0 GT/s speed may be dangerous

[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. Even if the odds are low, to contain any damage,
let's prevent 5.0 GT/s operation as well as add a comment to
Root Complex driver core, documenting this danger of data loss or
worse.

---

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/

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/

---

Geraldo Nascimento (2):
  PCI: rockchip: limit RK3399 to 2.5 GT/s to avoid data loss
  PCI: rockchip-host: comment danger of 5.0 GT/s speed

 drivers/pci/controller/pcie-rockchip-host.c |  4 ++++
 drivers/pci/controller/pcie-rockchip.c      | 10 ++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.52.0

[PATCH v3 1/2] PCI: rockchip: limit RK3399 to 2.5 GT/s to avoid data loss

[Geraldo Nascimento]

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

This may cause data loss or worse. Warn the user if they make a DT
change from the default and drive at 2.5 GT/s only, even if the DT
max-link-speed property is invalid or inexistent.

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>
Reviewed-by: Dragan Simic <dsimic@manjaro.org>
Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 drivers/pci/controller/pcie-rockchip.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
index 0f88da378805..25a6f67af839 100644
--- a/drivers/pci/controller/pcie-rockchip.c
+++ b/drivers/pci/controller/pcie-rockchip.c
@@ -66,8 +66,14 @@ 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) {
+		rockchip->link_gen = 1;
+		dev_warn(dev, "invalid max-link-speed, set to 2.5 GT/s\n");
+	}
+	else if (rockchip->link_gen == 2) {
+		rockchip->link_gen = 1;
+		dev_warn(dev, "5.0 GT/s may cause data loss, force 2.5 GT/s\n");
+	}
 
 	for (i = 0; i < ROCKCHIP_NUM_PM_RSTS; i++)
 		rockchip->pm_rsts[i].id = rockchip_pci_pm_rsts[i];
-- 
2.52.0

[PATCH v3 2/2] PCI: rockchip-host: comment danger of 5.0 GT/s speed

[Geraldo Nascimento]

According to Rockchip sources, there is grave danger in enabling 5.0
GT/s speed for this core. Add a comment documenting this risk of
"catastrophic failure" and discouraging end-users from forcing
higher speed.

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>
Reviewed-by: Dragan Simic <dsimic@manjaro.org>
Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 drivers/pci/controller/pcie-rockchip-host.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index ee1822ca01db..4faf3e29b7d5 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -332,6 +332,10 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
 		/*
 		 * Enable retrain for gen2. This should be configured only after
 		 * gen1 finished.
+		 *
+		 * Dangerous and may lead to "catastrophic failure", with data loss
+		 * or worse!
+		 *
 		 */
 		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
 		status &= ~PCI_EXP_LNKCTL2_TLS;
-- 
2.52.0

Re: [PATCH v3 2/2] PCI: rockchip-host: comment danger of 5.0 GT/s speed

[Bjorn Helgaas]

On Wed, Feb 25, 2026 at 07:27:44PM -0300, Geraldo Nascimento wrote:
> According to Rockchip sources, there is grave danger in enabling 5.0
> GT/s speed for this core. Add a comment documenting this risk of
> "catastrophic failure" and discouraging end-users from forcing
> higher speed.

A comment in the code might be useful but doesn't discourage end
users.

> 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>
> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> ---
>  drivers/pci/controller/pcie-rockchip-host.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> index ee1822ca01db..4faf3e29b7d5 100644
> --- a/drivers/pci/controller/pcie-rockchip-host.c
> +++ b/drivers/pci/controller/pcie-rockchip-host.c
> @@ -332,6 +332,10 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
>  		/*
>  		 * Enable retrain for gen2. This should be configured only after
>  		 * gen1 finished.
> +		 *
> +		 * Dangerous and may lead to "catastrophic failure", with data loss
> +		 * or worse!
> +		 *

Wrap comment to fit in 80 columns like the rest of the file.

Remove spurious blank line at end.

This should probably be squashed with the first patch; I don't see the
benefit of having them separate.

Why don't we just remove this whole "link_gen == 2" block?  If we
don't want to use it, there's no point in keeping the code.

>  		 */
>  		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
>  		status &= ~PCI_EXP_LNKCTL2_TLS;
> -- 
> 2.52.0
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v3 2/2] PCI: rockchip-host: comment danger of 5.0 GT/s speed

[Geraldo Nascimento]

On Wed, Feb 25, 2026 at 06:12:29PM -0600, Bjorn Helgaas wrote:
> On Wed, Feb 25, 2026 at 07:27:44PM -0300, Geraldo Nascimento wrote:
> > According to Rockchip sources, there is grave danger in enabling 5.0
> > GT/s speed for this core. Add a comment documenting this risk of
> > "catastrophic failure" and discouraging end-users from forcing
> > higher speed.
> 
> A comment in the code might be useful but doesn't discourage end
> users.

Hi Bjorn, thanks for the quick review.

> 
> > 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>
> > Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > ---
> >  drivers/pci/controller/pcie-rockchip-host.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> > index ee1822ca01db..4faf3e29b7d5 100644
> > --- a/drivers/pci/controller/pcie-rockchip-host.c
> > +++ b/drivers/pci/controller/pcie-rockchip-host.c
> > @@ -332,6 +332,10 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
> >  		/*
> >  		 * Enable retrain for gen2. This should be configured only after
> >  		 * gen1 finished.
> > +		 *
> > +		 * Dangerous and may lead to "catastrophic failure", with data loss
> > +		 * or worse!
> > +		 *
> 
> Wrap comment to fit in 80 columns like the rest of the file.
> 
> Remove spurious blank line at end.
> 
> This should probably be squashed with the first patch; I don't see the
> benefit of having them separate.
> 
> Why don't we just remove this whole "link_gen == 2" block?  If we
> don't want to use it, there's no point in keeping the code.

Yes, I think I'll go with this second option and remove the code
block outright.

Thanks,
Geraldo Nascimento

> 
> >  		 */
> >  		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
> >  		status &= ~PCI_EXP_LNKCTL2_TLS;
> > -- 
> > 2.52.0
> > 
> > 
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v3 2/2] PCI: rockchip-host: comment danger of 5.0 GT/s speed

[Dragan Simic]

Hello Geraldo and Bjorn,

On Thursday, February 26, 2026 01:18 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> On Wed, Feb 25, 2026 at 06:12:29PM -0600, Bjorn Helgaas wrote:
> > On Wed, Feb 25, 2026 at 07:27:44PM -0300, Geraldo Nascimento wrote:
> > > According to Rockchip sources, there is grave danger in enabling 5.0
> > > GT/s speed for this core. Add a comment documenting this risk of
> > > "catastrophic failure" and discouraging end-users from forcing
> > > higher speed.
> > 
> > A comment in the code might be useful but doesn't discourage end
> > users.
> 
> Hi Bjorn, thanks for the quick review.
> 
> > > 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>
> > > Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> > > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > > ---
> > >  drivers/pci/controller/pcie-rockchip-host.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> > > index ee1822ca01db..4faf3e29b7d5 100644
> > > --- a/drivers/pci/controller/pcie-rockchip-host.c
> > > +++ b/drivers/pci/controller/pcie-rockchip-host.c
> > > @@ -332,6 +332,10 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
> > >  		/*
> > >  		 * Enable retrain for gen2. This should be configured only after
> > >  		 * gen1 finished.
> > > +		 *
> > > +		 * Dangerous and may lead to "catastrophic failure", with data loss
> > > +		 * or worse!
> > > +		 *
> > 
> > Wrap comment to fit in 80 columns like the rest of the file.
> > 
> > Remove spurious blank line at end.
> > 
> > This should probably be squashed with the first patch; I don't see the
> > benefit of having them separate.
> > 
> > Why don't we just remove this whole "link_gen == 2" block?  If we
> > don't want to use it, there's no point in keeping the code.
> 
> Yes, I think I'll go with this second option and remove the code
> block outright.

I agree about removing the effectively unused code block that handles
the "link_gen == 2" case from pcie-rockchip-host.c, but then please
remove the PCIE_CLIENT_GEN_SEL_2 define from pcie-rockchip.h and its
single "link_gen == 2" use from pcie-rockchip.c as well.

As part of that additional removal, an early check for "link_gen == 1"
should be added to function rockchip_pcie_init_port() in pcie-rockchip.c,
because that will become the only "link_gen" value it supports.

> > 
> > >  		 */
> > >  		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
> > >  		status &= ~PCI_EXP_LNKCTL2_TLS;

Re: [PATCH v3 2/2] PCI: rockchip-host: comment danger of 5.0 GT/s speed

[Geraldo Nascimento]

Hi Dragan, thank you for the review!

On Thu, Feb 26, 2026 at 05:52:58AM +0100, Dragan Simic wrote:
> I agree about removing the effectively unused code block that handles
> the "link_gen == 2" case from pcie-rockchip-host.c, but then please
> remove the PCIE_CLIENT_GEN_SEL_2 define from pcie-rockchip.h and its
> single "link_gen == 2" use from pcie-rockchip.c as well.

Good.

There's also some code that needs to be dropped from the endpoint
driver.

> 
> As part of that additional removal, an early check for "link_gen == 1"
> should be added to function rockchip_pcie_init_port() in pcie-rockchip.c,
> because that will become the only "link_gen" value it supports.

What do you think, should the driver bail out completely if
link_gen != 1 or just let it force 2.5 GT/s speed with a warning?

I tend to go with the latter.

Thanks,
Geraldo Nascimento

Re: [PATCH v3 2/2] PCI: rockchip-host: comment danger of 5.0 GT/s speed

[Dragan Simic]

On Thursday, February 26, 2026 06:06 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> On Thu, Feb 26, 2026 at 05:52:58AM +0100, Dragan Simic wrote:
> > I agree about removing the effectively unused code block that handles
> > the "link_gen == 2" case from pcie-rockchip-host.c, but then please
> > remove the PCIE_CLIENT_GEN_SEL_2 define from pcie-rockchip.h and its
> > single "link_gen == 2" use from pcie-rockchip.c as well.
> 
> Good.
> 
> There's also some code that needs to be dropped from the endpoint
> driver.

Oh indeed, there's a redundant code block that handles a "link_gen == 2"
case in function rockchip_pcie_ep_link_training().  As a note, a similar
early check against "link_gen == 2" should be added there as well.

> > As part of that additional removal, an early check for "link_gen == 1"
> > should be added to function rockchip_pcie_init_port() in pcie-rockchip.c,
> > because that will become the only "link_gen" value it supports.
> 
> What do you think, should the driver bail out completely if
> link_gen != 1 or just let it force 2.5 GT/s speed with a warning?
> 
> I tend to go with the latter.

Actually, I'd go with the first option, i.e. bailing out early in function
rockchip_pcie_host_init_port() with an error message emitted, because that
function isn't supposed to define the link parameters.  Thus, if it ends up
receiving wrong link parameters, the issue lies somewhere else and should
be fixed at its origin.