Thread (1 message) 1 message, 1 author, 2025-08-05
STALE397d REVIEWED: 34 (34M)

4 review trailers (2 from subsystem maintainers).

[PATCH AUTOSEL 6.16-6.15] phy: rockchip-pcie: Enable all four lanes if required

From: Sasha Levin <sashal@kernel.org>
Date: 2025-08-05 13:10:59
Also in: linux-patches, linux-rockchip, stable
Subsystem: arm/rockchip soc support, generic phy framework, the rest · Maintainers: Heiko Stuebner, Vinod Koul, Linus Torvalds

From: Valmantas Paliksa <redacted>

[ 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 <redacted>
Signed-off-by: Geraldo Nascimento <redacted>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Link: https://lore.kernel.org/r/16b610aab34e069fd31d9f57260c10df2a968f80.1751322015.git.geraldogabriel@gmail.com (local)
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
-- 
2.39.5

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help