Re: [PATCH v4 2/4] PCI:?==?utf-8?q? 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 at 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 at vger.kernel.org
> Reported-by: Dragan Simic <dsimic at manjaro.org>
> Reported-by: Shawn Lin <shawn.lin at rock-chips.com>
> Signed-off-by: Geraldo Nascimento <geraldogabriel at 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.