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

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

[Geraldo Nascimento]

In recent interactions with Shawn Lin from Rockchip it came to my
attention there's an unknown errata regarding 5.0 GT/s operational
speed of their PCIe core. According to Shawn there's grave danger
even if the odds are low. To contain any damage, let's cover the
remaining corner-cases where the default would lead to 5.0 GT/s
operation as well as add a comment to Root Complex driver core,
documenting this danger.

Geraldo Nascimento (3):
  PCI: rockchip: warn of danger of 5.0 GT/s speeds
  PCI: rockchip-host: comment danger of 5.0 GT/s speed
  arm64: dts: rockchip: drop max-link-speed = <2> in helios64 PCIe

 arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts | 1 -
 drivers/pci/controller/pcie-rockchip-host.c            | 5 +++++
 drivers/pci/controller/pcie-rockchip.c                 | 8 ++++++--
 3 files changed, 11 insertions(+), 3 deletions(-)

-- 
2.49.0

[PATCH 1/3] PCI: rockchip: warn of danger of 5.0 GT/s speeds

[Geraldo Nascimento]

Shawn Lin from Rockchip has reiterated that there may be danger in using
their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change
from the default and at the same time, drive at 2.5 GT/s only, in case
the DT max-link-speed property is invalid or inexistent.

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

diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
index 0f88da378805..ed67886a6d43 100644
--- a/drivers/pci/controller/pcie-rockchip.c
+++ b/drivers/pci/controller/pcie-rockchip.c
@@ -66,8 +66,12 @@ 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)
+		dev_warn(dev, "5.0 GT/s may lead to catastrophic failure\n");
 
 	for (i = 0; i < ROCKCHIP_NUM_PM_RSTS; i++)
 		rockchip->pm_rsts[i].id = rockchip_pci_pm_rsts[i];
-- 
2.49.0

Re: [PATCH 1/3] PCI: rockchip: warn of danger of 5.0 GT/s speeds

[Dragan Simic]

Hello Geraldo,

On Saturday, November 15, 2025 10:10 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> Shawn Lin from Rockchip has reiterated that there may be danger in using
> their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change
> from the default and at the same time, drive at 2.5 GT/s only, in case
> the DT max-link-speed property is invalid or inexistent.
> 
> Fixes: 956cd99b35a8 ("PCI: rockchip: Separate common code from RC driver")
> Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> ---
>  drivers/pci/controller/pcie-rockchip.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
> index 0f88da378805..ed67886a6d43 100644
> --- a/drivers/pci/controller/pcie-rockchip.c
> +++ b/drivers/pci/controller/pcie-rockchip.c
> @@ -66,8 +66,12 @@ 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)
> +		dev_warn(dev, "5.0 GT/s may lead to catastrophic failure\n");
>  
>  	for (i = 0; i < ROCKCHIP_NUM_PM_RSTS; i++)
>  		rockchip->pm_rsts[i].id = rockchip_pci_pm_rsts[i];

Looking good to me, thanks for this patch!  People often declare
PCIe Gen2 on RK3399-based boards as "works for me", which actually
happens very often, but such simple evaluations cannot be taken
as a proof of Gen2 correctness.

Furthermore, RK3399's internal interconnects limit the effective
I/O speed of PCIe transfers already, so switching to PCIe Gen1
actually doesn't result in some large I/O performance penalties.

With all that in mind, please feel free to include

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

Re: [PATCH 1/3] PCI: rockchip: warn of danger of 5.0 GT/s speeds

[Geraldo Nascimento]

On Sat, Nov 15, 2025 at 10:24:49AM +0100, Dragan Simic wrote:
> Hello Geraldo,
> 
> On Saturday, November 15, 2025 10:10 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > Shawn Lin from Rockchip has reiterated that there may be danger in using
> > their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change
> > from the default and at the same time, drive at 2.5 GT/s only, in case
> > the DT max-link-speed property is invalid or inexistent.
> > 
> > Fixes: 956cd99b35a8 ("PCI: rockchip: Separate common code from RC driver")
> > Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> > Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > ---
> >  drivers/pci/controller/pcie-rockchip.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
> > index 0f88da378805..ed67886a6d43 100644
> > --- a/drivers/pci/controller/pcie-rockchip.c
> > +++ b/drivers/pci/controller/pcie-rockchip.c
> > @@ -66,8 +66,12 @@ 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)
> > +		dev_warn(dev, "5.0 GT/s may lead to catastrophic failure\n");
> >  
> >  	for (i = 0; i < ROCKCHIP_NUM_PM_RSTS; i++)
> >  		rockchip->pm_rsts[i].id = rockchip_pci_pm_rsts[i];
> 
> Looking good to me, thanks for this patch!  People often declare
> PCIe Gen2 on RK3399-based boards as "works for me", which actually
> happens very often, but such simple evaluations cannot be taken
> as a proof of Gen2 correctness.
> 
> Furthermore, RK3399's internal interconnects limit the effective
> I/O speed of PCIe transfers already, so switching to PCIe Gen1
> actually doesn't result in some large I/O performance penalties.
> 
> With all that in mind, please feel free to include
> 
> Reviewed-by: Dragan Simic <dsimic@manjaro.org>

Hi Dragan,

I think I'll include a Reported-By: you too, since you were the first to
warn me of unknown errata regarding Rockchip PCIe.

I told you it was a test of mine but I didn't realize the grave danger
implied by Shawn Lin declarations.

Once I realized this could end badly the only way I could retribute was
to send a few corrections :)

Thank you,
Geraldo Nascimento

[PATCH 2/3] 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 that danger and
discouraging end-users from forcing higher speed through DT changes.

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

diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index ee1822ca01db..7e6ff76466b7 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -332,6 +332,11 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
 		/*
 		 * Enable retrain for gen2. This should be configured only after
 		 * gen1 finished.
+		 *
+		 * According to Rockchip this path is dangerous and may lead to
+		 * catastrophic failure. Even if the odds are small, users are
+		 * still discouraged to engage the corresponding DT option.
+		 *
 		 */
 		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
 		status &= ~PCI_EXP_LNKCTL2_TLS;
-- 
2.49.0

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

[Dragan Simic]

Hello Geraldo,

On Saturday, November 15, 2025 10:10 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> According to Rockchip sources, there is grave danger in enabling 5.0
> GT/s speed for this core. Add a comment documenting that danger and
> discouraging end-users from forcing higher speed through DT changes.
> 
> Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> ---
>  drivers/pci/controller/pcie-rockchip-host.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> index ee1822ca01db..7e6ff76466b7 100644
> --- a/drivers/pci/controller/pcie-rockchip-host.c
> +++ b/drivers/pci/controller/pcie-rockchip-host.c
> @@ -332,6 +332,11 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
>  		/*
>  		 * Enable retrain for gen2. This should be configured only after
>  		 * gen1 finished.
> +		 *
> +		 * According to Rockchip this path is dangerous and may lead to
> +		 * catastrophic failure. Even if the odds are small, users are
> +		 * still discouraged to engage the corresponding DT option.
> +		 *
>  		 */
>  		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
>  		status &= ~PCI_EXP_LNKCTL2_TLS;

Looking good to me, thanks for this patch!  There's no need
to emit warnings here, because they'd be emitted already in
the rockchip_pcie_parse_dt() function.

Please feel free to include

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

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

[Geraldo Nascimento]

On Sat, Nov 15, 2025 at 10:30:49AM +0100, Dragan Simic wrote:
> Hello Geraldo,
> 
> On Saturday, November 15, 2025 10:10 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > According to Rockchip sources, there is grave danger in enabling 5.0
> > GT/s speed for this core. Add a comment documenting that danger and
> > discouraging end-users from forcing higher speed through DT changes.
> > 
> > Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> > Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > ---
> >  drivers/pci/controller/pcie-rockchip-host.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> > index ee1822ca01db..7e6ff76466b7 100644
> > --- a/drivers/pci/controller/pcie-rockchip-host.c
> > +++ b/drivers/pci/controller/pcie-rockchip-host.c
> > @@ -332,6 +332,11 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
> >  		/*
> >  		 * Enable retrain for gen2. This should be configured only after
> >  		 * gen1 finished.
> > +		 *
> > +		 * According to Rockchip this path is dangerous and may lead to
> > +		 * catastrophic failure. Even if the odds are small, users are
> > +		 * still discouraged to engage the corresponding DT option.
> > +		 *
> >  		 */
> >  		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
> >  		status &= ~PCI_EXP_LNKCTL2_TLS;
> 
> Looking good to me, thanks for this patch!  There's no need
> to emit warnings here, because they'd be emitted already in
> the rockchip_pcie_parse_dt() function.
> 
> Please feel free to include
> 
> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
>

I disagree, I think the comment stands.

Even if we reduce to one line, ex:

+ May cause damage

Regards,
Geraldo Nascimento

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

[Dragan Simic]

On Saturday, November 15, 2025 10:51 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> On Sat, Nov 15, 2025 at 10:30:49AM +0100, Dragan Simic wrote:
> > On Saturday, November 15, 2025 10:10 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > > According to Rockchip sources, there is grave danger in enabling 5.0
> > > GT/s speed for this core. Add a comment documenting that danger and
> > > discouraging end-users from forcing higher speed through DT changes.
> > > 
> > > Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> > > Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> > > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > > ---
> > >  drivers/pci/controller/pcie-rockchip-host.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> > > index ee1822ca01db..7e6ff76466b7 100644
> > > --- a/drivers/pci/controller/pcie-rockchip-host.c
> > > +++ b/drivers/pci/controller/pcie-rockchip-host.c
> > > @@ -332,6 +332,11 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
> > >  		/*
> > >  		 * Enable retrain for gen2. This should be configured only after
> > >  		 * gen1 finished.
> > > +		 *
> > > +		 * According to Rockchip this path is dangerous and may lead to
> > > +		 * catastrophic failure. Even if the odds are small, users are
> > > +		 * still discouraged to engage the corresponding DT option.
> > > +		 *
> > >  		 */
> > >  		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
> > >  		status &= ~PCI_EXP_LNKCTL2_TLS;
> > 
> > Looking good to me, thanks for this patch!  There's no need
> > to emit warnings here, because they'd be emitted already in
> > the rockchip_pcie_parse_dt() function.
> > 
> > Please feel free to include
> > 
> > Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> >
> 
> I disagree, I think the comment stands.
> 
> Even if we reduce to one line, ex:
> 
> + May cause damage

Ah, perhaps I wasn't clear enough, so let me clarify a bit.  The
comment you added is fine, I just referred to no need for emitting
a warning at that point, because it would be emitted already.

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

[Geraldo Nascimento]

On Sat, Nov 15, 2025 at 11:01:21AM +0100, Dragan Simic wrote:
> On Saturday, November 15, 2025 10:51 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > On Sat, Nov 15, 2025 at 10:30:49AM +0100, Dragan Simic wrote:
> > > Looking good to me, thanks for this patch!  There's no need
> > > to emit warnings here, because they'd be emitted already in
> > > the rockchip_pcie_parse_dt() function.
> > > 
> > > Please feel free to include
> > > 
> > > Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> > >
> > 
> > I disagree, I think the comment stands.
> > 
> > Even if we reduce to one line, ex:
> > 
> > + May cause damage
> 
> Ah, perhaps I wasn't clear enough, so let me clarify a bit.  The
> comment you added is fine, I just referred to no need for emitting
> a warning at that point, because it would be emitted already.

OK, I get it now so I think it's time to send v2 with all that in mind
:)

Thanks,
Geraldo Nascimento

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

[Dragan Simic]

On Saturday, November 15, 2025 11:04 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> On Sat, Nov 15, 2025 at 11:01:21AM +0100, Dragan Simic wrote:
> > On Saturday, November 15, 2025 10:51 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > > On Sat, Nov 15, 2025 at 10:30:49AM +0100, Dragan Simic wrote:
> > > > Looking good to me, thanks for this patch!  There's no need
> > > > to emit warnings here, because they'd be emitted already in
> > > > the rockchip_pcie_parse_dt() function.
> > > > 
> > > > Please feel free to include
> > > > 
> > > > Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> > > >
> > > 
> > > I disagree, I think the comment stands.
> > > 
> > > Even if we reduce to one line, ex:
> > > 
> > > + May cause damage
> > 
> > Ah, perhaps I wasn't clear enough, so let me clarify a bit.  The
> > comment you added is fine, I just referred to no need for emitting
> > a warning at that point, because it would be emitted already.
> 
> OK, I get it now so I think it's time to send v2 with all that in mind
> :)

Please, wait a day or two before sending the v2, because that's
standard procedure.  Sending more than one version in the same day
is highly discouraged, because it doesn't give enough time to people
for reviewing, and may cause people to look at a wrong version.

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

[Geraldo Nascimento]

On Sat, Nov 15, 2025 at 11:09:42AM +0100, Dragan Simic wrote:
> Please, wait a day or two before sending the v2, because that's
> standard procedure.  Sending more than one version in the same day
> is highly discouraged, because it doesn't give enough time to people
> for reviewing, and may cause people to look at a wrong version.

Yeah, you're right, sorry.

[PATCH 3/3] arm64: dts: rockchip: drop max-link-speed = <2> in helios64 PCIe

[Geraldo Nascimento]

Shawn Lin from Rockchip strongly discourages attempts to use their
RK3399 PCIe core at 5.0 GT/s speed, citing concerns about catastrophic
failures that may happen. Even if the odds are low, drop from last user
of this property for the RK3399 platform, helios64.

Fixes: 755fff528b1b ("arm64: dts: rockchip: add variables for pcie completion to helios64")
Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts b/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
index e7d4a2f9a95e..78a7775c3b22 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
@@ -424,7 +424,6 @@ &pcie_phy {
 
 &pcie0 {
 	ep-gpios = <&gpio2 RK_PD4 GPIO_ACTIVE_HIGH>;
-	max-link-speed = <2>;
 	num-lanes = <2>;
 	pinctrl-names = "default";
 	status = "okay";
-- 
2.49.0

Re: [PATCH 3/3] arm64: dts: rockchip: drop max-link-speed = <2> in helios64 PCIe

[Dragan Simic]

Hello Geraldo,

On Saturday, November 15, 2025 10:10 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> Shawn Lin from Rockchip strongly discourages attempts to use their
> RK3399 PCIe core at 5.0 GT/s speed, citing concerns about catastrophic
> failures that may happen. Even if the odds are low, drop from last user
> of this property for the RK3399 platform, helios64.
> 
> Fixes: 755fff528b1b ("arm64: dts: rockchip: add variables for pcie completion to helios64")
> Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts b/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> index e7d4a2f9a95e..78a7775c3b22 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> @@ -424,7 +424,6 @@ &pcie_phy {
>  
>  &pcie0 {
>  	ep-gpios = <&gpio2 RK_PD4 GPIO_ACTIVE_HIGH>;
> -	max-link-speed = <2>;
>  	num-lanes = <2>;
>  	pinctrl-names = "default";
>  	status = "okay";

Looking good to me, this rounds up the prevention of issues
coming from buggy PCIe Gen2 on RK3399.

Please feel free to include

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

Though, could you, please, add patch 4/3 to this series, which
would remove the redundant parameter "max-link-speed = <1>" from
rk3399-nanopi-r4s.dtsi?

Re: [PATCH 3/3] arm64: dts: rockchip: drop max-link-speed = <2> in helios64 PCIe

[Dragan Simic]

On Saturday, November 15, 2025 10:36 CET, "Dragan Simic" <dsimic@manjaro.org> wrote:
> On Saturday, November 15, 2025 10:10 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > Shawn Lin from Rockchip strongly discourages attempts to use their
> > RK3399 PCIe core at 5.0 GT/s speed, citing concerns about catastrophic
> > failures that may happen. Even if the odds are low, drop from last user
> > of this property for the RK3399 platform, helios64.
> > 
> > Fixes: 755fff528b1b ("arm64: dts: rockchip: add variables for pcie completion to helios64")
> > Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> > Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts b/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> > index e7d4a2f9a95e..78a7775c3b22 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> > @@ -424,7 +424,6 @@ &pcie_phy {
> >  
> >  &pcie0 {
> >  	ep-gpios = <&gpio2 RK_PD4 GPIO_ACTIVE_HIGH>;
> > -	max-link-speed = <2>;
> >  	num-lanes = <2>;
> >  	pinctrl-names = "default";
> >  	status = "okay";
> 
> Looking good to me, this rounds up the prevention of issues
> coming from buggy PCIe Gen2 on RK3399.
> 
> Please feel free to include
> 
> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> 
> Though, could you, please, add patch 4/3 to this series, which
> would remove the redundant parameter "max-link-speed = <1>" from
> rk3399-nanopi-r4s.dtsi?

Sorry, I forgot to note that the patch summary would read a bit
better if it were reworded like this:

  arm64: dts: rockchip: Remove redundant max-link-speed from helios64 board dts

Re: [PATCH 3/3] arm64: dts: rockchip: drop max-link-speed = <2> in helios64 PCIe

[Geraldo Nascimento]

On Sat, Nov 15, 2025 at 10:42:40AM +0100, Dragan Simic wrote:
> On Saturday, November 15, 2025 10:36 CET, "Dragan Simic" <dsimic@manjaro.org> wrote:
> > On Saturday, November 15, 2025 10:10 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > > Shawn Lin from Rockchip strongly discourages attempts to use their
> > > RK3399 PCIe core at 5.0 GT/s speed, citing concerns about catastrophic
> > > failures that may happen. Even if the odds are low, drop from last user
> > > of this property for the RK3399 platform, helios64.
> > > 
> > > Fixes: 755fff528b1b ("arm64: dts: rockchip: add variables for pcie completion to helios64")
> > > Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> > > Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> > > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > > ---
> > >  arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts | 1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts b/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> > > index e7d4a2f9a95e..78a7775c3b22 100644
> > > --- a/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> > > @@ -424,7 +424,6 @@ &pcie_phy {
> > >  
> > >  &pcie0 {
> > >  	ep-gpios = <&gpio2 RK_PD4 GPIO_ACTIVE_HIGH>;
> > > -	max-link-speed = <2>;
> > >  	num-lanes = <2>;
> > >  	pinctrl-names = "default";
> > >  	status = "okay";
> > 
> > Looking good to me, this rounds up the prevention of issues
> > coming from buggy PCIe Gen2 on RK3399.
> > 
> > Please feel free to include
> > 
> > Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> > 
> > Though, could you, please, add patch 4/3 to this series, which
> > would remove the redundant parameter "max-link-speed = <1>" from
> > rk3399-nanopi-r4s.dtsi?
> 
> Sorry, I forgot to note that the patch summary would read a bit
> better if it were reworded like this:
> 
>   arm64: dts: rockchip: Remove redundant max-link-speed from helios64 board dts
>

No, I think:

arm64: dts: rockchip: Remove dangerous max-link-speed from helios64 board dts

Is more accurately described. With focus on the dangerous, we're not
liable.

Redudant max-link-speed only for nanopi-r4s include definitions I think!

Thanks,
Geraldo Nascimento

Re: [PATCH 3/3] arm64: dts: rockchip: drop max-link-speed = <2> in helios64 PCIe

[Dragan Simic]

On Saturday, November 15, 2025 10:55 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> On Sat, Nov 15, 2025 at 10:42:40AM +0100, Dragan Simic wrote:
> > On Saturday, November 15, 2025 10:36 CET, "Dragan Simic" <dsimic@manjaro.org> wrote:
> > > On Saturday, November 15, 2025 10:10 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > > > Shawn Lin from Rockchip strongly discourages attempts to use their
> > > > RK3399 PCIe core at 5.0 GT/s speed, citing concerns about catastrophic
> > > > failures that may happen. Even if the odds are low, drop from last user
> > > > of this property for the RK3399 platform, helios64.
> > > > 
> > > > Fixes: 755fff528b1b ("arm64: dts: rockchip: add variables for pcie completion to helios64")
> > > > Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> > > > Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> > > > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > > > ---
> > > >  arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts | 1 -
> > > >  1 file changed, 1 deletion(-)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts b/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> > > > index e7d4a2f9a95e..78a7775c3b22 100644
> > > > --- a/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> > > > +++ b/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> > > > @@ -424,7 +424,6 @@ &pcie_phy {
> > > >  
> > > >  &pcie0 {
> > > >  	ep-gpios = <&gpio2 RK_PD4 GPIO_ACTIVE_HIGH>;
> > > > -	max-link-speed = <2>;
> > > >  	num-lanes = <2>;
> > > >  	pinctrl-names = "default";
> > > >  	status = "okay";
> > > 
> > > Looking good to me, this rounds up the prevention of issues
> > > coming from buggy PCIe Gen2 on RK3399.
> > > 
> > > Please feel free to include
> > > 
> > > Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> > > 
> > > Though, could you, please, add patch 4/3 to this series, which
> > > would remove the redundant parameter "max-link-speed = <1>" from
> > > rk3399-nanopi-r4s.dtsi?
> > 
> > Sorry, I forgot to note that the patch summary would read a bit
> > better if it were reworded like this:
> > 
> >   arm64: dts: rockchip: Remove redundant max-link-speed from helios64 board dts
> 
> No, I think:
> 
> arm64: dts: rockchip: Remove dangerous max-link-speed from helios64 board dts
> 
> Is more accurately described. With focus on the dangerous, we're not
> liable.
> 
> Redudant max-link-speed only for nanopi-r4s include definitions I think!

Indeed, you're right.  It was obviously an -ENOCOFFEE on my part. :)

Re: [PATCH 3/3] arm64: dts: rockchip: drop max-link-speed = <2> in helios64 PCIe

[Geraldo Nascimento]

On Sat, Nov 15, 2025 at 10:36:17AM +0100, Dragan Simic wrote:
> Hello Geraldo,
> 
> On Saturday, November 15, 2025 10:10 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > Shawn Lin from Rockchip strongly discourages attempts to use their
> > RK3399 PCIe core at 5.0 GT/s speed, citing concerns about catastrophic
> > failures that may happen. Even if the odds are low, drop from last user
> > of this property for the RK3399 platform, helios64.
> > 
> > Fixes: 755fff528b1b ("arm64: dts: rockchip: add variables for pcie completion to helios64")
> > Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> > Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts b/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> > index e7d4a2f9a95e..78a7775c3b22 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> > @@ -424,7 +424,6 @@ &pcie_phy {
> >  
> >  &pcie0 {
> >  	ep-gpios = <&gpio2 RK_PD4 GPIO_ACTIVE_HIGH>;
> > -	max-link-speed = <2>;
> >  	num-lanes = <2>;
> >  	pinctrl-names = "default";
> >  	status = "okay";
> 
> Looking good to me, this rounds up the prevention of issues
> coming from buggy PCIe Gen2 on RK3399.
> 
> Please feel free to include
> 
> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> 
> Though, could you, please, add patch 4/3 to this series, which
> would remove the redundant parameter "max-link-speed = <1>" from
> rk3399-nanopi-r4s.dtsi?
>

Thanks for catch, will certainly be included in v2 after I get a few
more reviews.

Geraldo Nascimento

Re: [PATCH 0/3] PCI: rockchip: 5.0 GT/s speed may be dangerous

[Shawn Lin]

Hi Geraldo,

在 2025/11/15 星期六 17:10, Geraldo Nascimento 写道:
> In recent interactions with Shawn Lin from Rockchip it came to my
> attention there's an unknown errata regarding 5.0 GT/s operational
> speed of their PCIe core. According to Shawn there's grave danger
> even if the odds are low. To contain any damage, let's cover the
> remaining corner-cases where the default would lead to 5.0 GT/s
> operation as well as add a comment to Root Complex driver core,
> documenting this danger.
> 

I'm not sure just adding a warn would be a good choice. Could we totally
force to use gen1 and add a warn if trying to use Gen2.

Meanwhile amend the commit message to add a reference
of RK3399 official datesheet[1] which says PCIe on RK3399 should only
support 2.5GT/s?


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

> Geraldo Nascimento (3):
>    PCI: rockchip: warn of danger of 5.0 GT/s speeds
>    PCI: rockchip-host: comment danger of 5.0 GT/s speed
>    arm64: dts: rockchip: drop max-link-speed = <2> in helios64 PCIe
> 
>   arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts | 1 -
>   drivers/pci/controller/pcie-rockchip-host.c            | 5 +++++
>   drivers/pci/controller/pcie-rockchip.c                 | 8 ++++++--
>   3 files changed, 11 insertions(+), 3 deletions(-)
> 

Re: [PATCH 0/3] PCI: rockchip: 5.0 GT/s speed may be dangerous

[Dragan Simic]

Hello Shawn and Geraldo,

On Monday, November 17, 2025 04:42 CET, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> 在 2025/11/15 星期六 17:10, Geraldo Nascimento 写道:
> > In recent interactions with Shawn Lin from Rockchip it came to my
> > attention there's an unknown errata regarding 5.0 GT/s operational
> > speed of their PCIe core. According to Shawn there's grave danger
> > even if the odds are low. To contain any damage, let's cover the
> > remaining corner-cases where the default would lead to 5.0 GT/s
> > operation as well as add a comment to Root Complex driver core,
> > documenting this danger.
> 
> I'm not sure just adding a warn would be a good choice. Could we totally
> force to use gen1 and add a warn if trying to use Gen2.

I think that forcing 2.5 GT/s with an appropriate warning message
is a good idea.  That would be like some quirk that gets applied
automatically, to prevent data corruption, while warning people
who attempt to "overclock" the PCIe interface.

> Meanwhile amend the commit message to add a reference
> of RK3399 official datesheet[1] which says PCIe on RK3399 should only
> support 2.5GT/s?
> 
> [1]https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf

Also, rewording the patch summary as follows below may be good,
because that would provide more details:

  PCI: rockchip: Warn about Gen2 5.0 GT/s on RK3399 being unsafe

Or, if we'll go with the automatic downgrading, like this:

  PCI: rockchip: Limit RK3399 to Gen1 2.5 GT/s to prevent breakage

Re: [PATCH 0/3] PCI: rockchip: 5.0 GT/s speed may be dangerous

[Geraldo Nascimento]

On Mon, Nov 17, 2025 at 04:57:11AM +0100, Dragan Simic wrote:
> Hello Shawn and Geraldo,
> 
> On Monday, November 17, 2025 04:42 CET, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> > 在 2025/11/15 星期六 17:10, Geraldo Nascimento 写道:
> > > In recent interactions with Shawn Lin from Rockchip it came to my
> > > attention there's an unknown errata regarding 5.0 GT/s operational
> > > speed of their PCIe core. According to Shawn there's grave danger
> > > even if the odds are low. To contain any damage, let's cover the
> > > remaining corner-cases where the default would lead to 5.0 GT/s
> > > operation as well as add a comment to Root Complex driver core,
> > > documenting this danger.
> > 
> > I'm not sure just adding a warn would be a good choice. Could we totally
> > force to use gen1 and add a warn if trying to use Gen2.
> 
> I think that forcing 2.5 GT/s with an appropriate warning message
> is a good idea.  That would be like some quirk that gets applied
> automatically, to prevent data corruption, while warning people
> who attempt to "overclock" the PCIe interface.

Hi Shawn and Dragan,

Alright, I'll send v2 with this suggestion in mind. So that driving the
core at 5.0 GT/s will require patching and compiling own kernel.

> 
> > Meanwhile amend the commit message to add a reference
> > of RK3399 official datesheet[1] which says PCIe on RK3399 should only
> > support 2.5GT/s?
> > 
> > [1]https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
> 

Shawn, URLs have the bad habit of changing or simply disappearing, so I
don't think it's a good idea to put URL in the commit message.

Also, the datasheet just mentions that RK3399 supports only 2.5 GT/s,
it does not mention possible damage from driving the core at 5.0 GT/s.

> Also, rewording the patch summary as follows below may be good,
> because that would provide more details:
> 
>   PCI: rockchip: Warn about Gen2 5.0 GT/s on RK3399 being unsafe
> 
> Or, if we'll go with the automatic downgrading, like this:
> 
>   PCI: rockchip: Limit RK3399 to Gen1 2.5 GT/s to prevent breakage
> 

Dragan, these are good ones, thanks. Though I think I'll omit Gen1/Gen2
wording since I know how much Bjorn dislikes those terms.

Thanks,
Geraldo Nascimento

Re: [PATCH 0/3] PCI: rockchip: 5.0 GT/s speed may be dangerous

[Dragan Simic]

Hello Geraldo,

On Monday, November 17, 2025 08:02 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> On Mon, Nov 17, 2025 at 04:57:11AM +0100, Dragan Simic wrote:
> > On Monday, November 17, 2025 04:42 CET, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> > > 在 2025/11/15 星期六 17:10, Geraldo Nascimento 写道:
> > > > In recent interactions with Shawn Lin from Rockchip it came to my
> > > > attention there's an unknown errata regarding 5.0 GT/s operational
> > > > speed of their PCIe core. According to Shawn there's grave danger
> > > > even if the odds are low. To contain any damage, let's cover the
> > > > remaining corner-cases where the default would lead to 5.0 GT/s
> > > > operation as well as add a comment to Root Complex driver core,
> > > > documenting this danger.
> > > 
> > > I'm not sure just adding a warn would be a good choice. Could we totally
> > > force to use gen1 and add a warn if trying to use Gen2.
> > 
> > I think that forcing 2.5 GT/s with an appropriate warning message
> > is a good idea.  That would be like some quirk that gets applied
> > automatically, to prevent data corruption, while warning people
> > who attempt to "overclock" the PCIe interface.
> 
> Alright, I'll send v2 with this suggestion in mind. So that driving the
> core at 5.0 GT/s will require patching and compiling own kernel.
> 
> > > Meanwhile amend the commit message to add a reference
> > > of RK3399 official datesheet[1] which says PCIe on RK3399 should only
> > > support 2.5GT/s?
> > > 
> > > [1]https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
> > 
> 
> Shawn, URLs have the bad habit of changing or simply disappearing, so I
> don't think it's a good idea to put URL in the commit message.

Ah, it's actually perfectly fine, there's always Wayback Machibe
to rescue references/URLs that may disappear over time.  Wikipedia
relies heavily on exactly that mechanism, for example.

[2] https://web.archive.org/

> Also, the datasheet just mentions that RK3399 supports only 2.5 GT/s,
> it does not mention possible damage from driving the core at 5.0 GT/s.

True, but having an additional reference doesn't hurt, especially
because the revision history mentions, albeit vaguely, an update
to the PCIe specification in version 1.1.of the datasheet.  Though,
if we had the version 1.0 publicly available for comparison, that
would've been much better. :)

> > Also, rewording the patch summary as follows below may be good,
> > because that would provide more details:
> > 
> >   PCI: rockchip: Warn about Gen2 5.0 GT/s on RK3399 being unsafe
> > 
> > Or, if we'll go with the automatic downgrading, like this:
> > 
> >   PCI: rockchip: Limit RK3399 to Gen1 2.5 GT/s to prevent breakage
> 
> Dragan, these are good ones, thanks. Though I think I'll omit Gen1/Gen2
> wording since I know how much Bjorn dislikes those terms.

I'm glad that you like those, and I also thought about not including
the GenX parts, because they're basically a bit redundant. :)