From: Geraldo Nascimento geraldogabriel@gmail.com
[ Upstream commit 25facbabc3fc33c794ad09d73f73268c0f8cbc7d ]
pcie_conf is used to touch TEST_WRITE strobe signal. This signal should be enabled, a little time waited, and then disabled. Current code clearly was copy-pasted and never disables the strobe signal. Adjust the define. While at it, remove PHY_CFG_RD_MASK which has been unused since 64cdc0360811 ("phy: rockchip-pcie: remove unused phy_rd_cfg function").
Reviewed-by: Neil Armstrong neil.armstrong@linaro.org Signed-off-by: Geraldo Nascimento geraldogabriel@gmail.com Link: https://lore.kernel.org/r/d514d5d5627680caafa8b7548cbdfee4307f5440.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 should be backported to stable kernel trees for the following reasons:
### Bug Analysis The commit fixes a clear programming bug where `PHY_CFG_WR_DISABLE` was incorrectly defined as `1` instead of `0`. Looking at the code in `phy_wr_cfg()` function (lines 100-120), the sequence is: 1. Write data and address to the configuration register 2. Wait 1 microsecond 3. Enable the TEST_WRITE strobe signal by writing `PHY_CFG_WR_ENABLE` (value 1) 4. Wait 1 microsecond 5. Intended to disable the strobe signal with `PHY_CFG_WR_DISABLE`
However, since both `PHY_CFG_WR_ENABLE` and `PHY_CFG_WR_DISABLE` were defined as `1`, the strobe signal was never actually disabled. This means the PHY configuration writes were leaving the strobe signal permanently enabled, which is incorrect hardware programming.
### Impact Assessment 1. **Real bug affecting users**: This is a functional bug that affects all Rockchip PCIe PHY operations. The strobe signal being left enabled could cause: - Incorrect PHY configuration behavior - Potential power consumption issues - Possible hardware state corruption - Unpredictable PCIe link behavior
2. **Small and contained fix**: The change is minimal - just changing one define from `1` to `0`. This is exactly the type of targeted fix suitable for stable.
3. **Clear correctness**: The fix is obviously correct - a disable operation should use value `0`, not `1`. The bug appears to be a copy-paste error from the initial driver introduction in 2016 (commit fcffee3d54fca).
4. **Long-standing issue**: This bug has existed since the driver was first introduced in 2016, affecting all kernel versions with this driver.
5. **No architectural changes**: The fix doesn't introduce new features or change the driver architecture - it simply corrects an incorrect constant value.
6. **Low regression risk**: Changing the disable value from 1 to 0 is the correct behavior according to typical hardware programming patterns. The risk of regression is minimal since this fixes incorrect behavior rather than changing working functionality.
### Additional Context The commit also removes the unused `PHY_CFG_RD_MASK` define as cleanup, which was left over from commit 64cdc0360811. This is harmless cleanup that doesn't affect the backport decision.
The commit message clearly describes the issue: the TEST_WRITE strobe signal should follow a pattern of enable→wait→disable, but the current code never actually disables it due to the incorrect define value.
This meets all the criteria for stable backports: it fixes a real bug, is small and self-contained, has minimal risk, and corrects clearly incorrect behavior that has been present since driver introduction.
drivers/phy/rockchip/phy-rockchip-pcie.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-pcie.c b/drivers/phy/rockchip/phy-rockchip-pcie.c index bd44af36c67a..63e88abc66c6 100644 --- a/drivers/phy/rockchip/phy-rockchip-pcie.c +++ b/drivers/phy/rockchip/phy-rockchip-pcie.c @@ -30,9 +30,8 @@ #define PHY_CFG_ADDR_SHIFT 1 #define PHY_CFG_DATA_MASK 0xf #define PHY_CFG_ADDR_MASK 0x3f -#define PHY_CFG_RD_MASK 0x3ff #define PHY_CFG_WR_ENABLE 1 -#define PHY_CFG_WR_DISABLE 1 +#define PHY_CFG_WR_DISABLE 0 #define PHY_CFG_WR_SHIFT 0 #define PHY_CFG_WR_MASK 1 #define PHY_CFG_PLL_LOCK 0x10