From: Valmantas Paliksa walmis@gmail.com
[ Upstream commit c3fe7071e196e25789ecf90dbc9e8491a98884d7 ]
Current code enables only Lane 0 because pwr_cnt will be incremented on first call to the function. Let's reorder the enablement code to enable all 4 lanes through GRF.
Reviewed-by: Neil Armstrong neil.armstrong@linaro.org Reviewed-by: Robin Murphy robin.murphy@arm.com
Signed-off-by: Valmantas Paliksa walmis@gmail.com Signed-off-by: Geraldo Nascimento geraldogabriel@gmail.com Reviewed-by: Robin Murphy robin.murphy@arm.com Reviewed-by: Neil Armstrong neil.armstrong@linaro.org Link: https://lore.kernel.org/r/16b610aab34e069fd31d9f57260c10df2a968f80.175132201... Signed-off-by: Vinod Koul vkoul@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
**Backport Status: YES**
This commit fixes a significant bug in the Rockchip PCIe PHY driver where only Lane 0 was being enabled instead of all required lanes. Here's my detailed analysis:
## Bug Description The original code had a critical logic error in `rockchip_pcie_phy_power_on()`. The lane enable operation (writing to `pcie_laneoff` register) was placed AFTER the `pwr_cnt++` check at line 170. Since `pwr_cnt` is a reference counter that tracks how many times the PHY has been powered on, the first call would increment it from 0 to 1 and continue with initialization. However, subsequent calls for other lanes (Lane 1, 2, 3) would hit the early return at line 171 (`goto err_out`), preventing those lanes from being enabled.
## The Fix The commit moves the lane enable operation (lines 184-188 in original) to BEFORE the `pwr_cnt++` check. This ensures that each lane gets properly enabled through the GRF (General Register File) regardless of the power reference count state.
## Why This Should Be Backported
1. **Fixes a Real Bug**: This fixes a functional bug where PCIe devices requiring multiple lanes (x2, x4 configurations) would only have Lane 0 enabled, severely impacting performance or causing complete failure to operate.
2. **Small and Contained Fix**: The change is minimal - just reordering 5 lines of code within a single function. No architectural changes or new features are introduced.
3. **Low Risk**: The fix simply ensures the lane enable register write happens for all lanes, which was clearly the original intent. The moved code block remains identical.
4. **Hardware Functionality Impact**: PCIe lane configuration is critical for proper hardware operation. Devices expecting x4 links but only getting x1 would experience significant performance degradation (75% bandwidth loss).
5. **Clear Root Cause**: The bug mechanism is straightforward - the reference counter was preventing lanes 1-3 from being configured due to early return.
6. **No Side Effects**: The change doesn't introduce new behavior, it just fixes the existing broken behavior to work as originally intended.
This is exactly the type of bug fix that stable kernels should receive - it's a clear functional regression fix with minimal code changes and low risk of introducing new issues.
drivers/phy/rockchip/phy-rockchip-pcie.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-pcie.c b/drivers/phy/rockchip/phy-rockchip-pcie.c index 63e88abc66c6..4e2dfd01adf2 100644 --- a/drivers/phy/rockchip/phy-rockchip-pcie.c +++ b/drivers/phy/rockchip/phy-rockchip-pcie.c @@ -159,6 +159,12 @@ static int rockchip_pcie_phy_power_on(struct phy *phy)
guard(mutex)(&rk_phy->pcie_mutex);
+ regmap_write(rk_phy->reg_base, + rk_phy->phy_data->pcie_laneoff, + HIWORD_UPDATE(!PHY_LANE_IDLE_OFF, + PHY_LANE_IDLE_MASK, + PHY_LANE_IDLE_A_SHIFT + inst->index)); + if (rk_phy->pwr_cnt++) { return 0; } @@ -175,12 +181,6 @@ static int rockchip_pcie_phy_power_on(struct phy *phy) PHY_CFG_ADDR_MASK, PHY_CFG_ADDR_SHIFT));
- regmap_write(rk_phy->reg_base, - rk_phy->phy_data->pcie_laneoff, - HIWORD_UPDATE(!PHY_LANE_IDLE_OFF, - PHY_LANE_IDLE_MASK, - PHY_LANE_IDLE_A_SHIFT + inst->index)); - /* * No documented timeout value for phy operation below, * so we make it large enough here. And we use loop-break