1. Patchew
  2. linux
  3. [PATCH v3 0/2] PCI: rockchip: 5.0 GT/s speed may be dangerous
  4. View patch

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

Geraldo Nascimento posted 2 patches 1 month, 1 week ago
[PATCH v3 2/2] PCI: rockchip-host: comment danger of 5.0 GT/s speed
Posted by Geraldo Nascimento 1 month, 1 week ago 
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
Posted by Bjorn Helgaas 1 month, 1 week ago 
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
Posted by Geraldo Nascimento 1 month, 1 week ago 
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
Posted by Dragan Simic 1 month, 1 week ago 
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
Posted by Geraldo Nascimento 1 month, 1 week ago 
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
Posted by Dragan Simic 1 month, 1 week ago 
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.

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.