Re: [PATCH v2 1/4] PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage

Hello Diederik,

On Thursday, December 18, 2025 11:01 CET, "Diederik de Haas" [off-list ref] wrote:

On Thu Dec 18, 2025 at 10:47 AM CET, Dragan Simic wrote:

quoted

On Thursday, December 18, 2025 09:05 CET, Manivannan Sadhasivam [off-list ref] wrote:

quoted

On Mon, Nov 17, 2025 at 06:47:05PM -0300, Geraldo Nascimento wrote:

quoted

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 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
says 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/ (local)
Cc: stable@vger.kernel.org
Reported-by: Dragan Simic <redacted>
Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
Reviewed-by: Dragan Simic <redacted>
Signed-off-by: Geraldo Nascimento <redacted>
---
 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..992ccf4b139e 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 is dangerous, set to 2.5 GT/s\n");

What does 'danger' really mean here? Link instability or something else?
Error messages should be precise and not fearmongering.

I agree that the original wording is a bit suboptimal, and I'd suggest
to Geraldo that the produced warning message is changed to

  "5.0 GT/s may cause data corruption, limited to to 2.5 GT/s\n"

or something similar, to better reflect the actual underlying issue.

s/limited to to/therefore limit speed to/ ?

That would work well in a book or an article, while slightly terse
wording is usually preferred in the messages produced by the kernel,
or in log messages in general.  Such an approach compacts as much
information as possible in as few words as possible, while still
remaining (mostly) grammatically correct.