[PATCH AUTOSEL 6.16] PCI: dw-rockchip: Delay link training after hot reset in EP mode

[PATCH AUTOSEL 6.16] PCI: dw-rockchip: Delay link training after hot reset in EP mode

[Sasha Levin]

From: Wilfred Mallawa <wilfred.mallawa@wdc.com>

[ Upstream commit c0b93754547dde16c8370b8fdad5f396e7786647 ]

RK3588 TRM, section "11.6.1.3.3 Hot Reset and Link-Down Reset" states that:

  If you want to delay link re-establishment (after reset) so that you can
  reprogram some registers through DBI, you must set app_ltssm_enable =0
  immediately after core_rst_n as shown in above. This can be achieved by
  enable the app_dly2_en, and end-up the delay by assert app_dly2_done.

I.e. setting app_dly2_en will automatically deassert app_ltssm_enable on
a hot reset, and setting app_dly2_done will re-assert app_ltssm_enable,
re-enabling link training.

When receiving a hot reset/link-down IRQ when running in EP mode, we will
call dw_pcie_ep_linkdown(), which may update registers through DBI. Unless
link training is inhibited, these register updates race with the link
training.

To avoid the race, set PCIE_LTSSM_APP_DLY2_EN so the controller never
automatically trains the link after a link-down or hot reset interrupt.
That way any DBI updates done in the dw_pcie_ep_linkdown() path will happen
while the link is still down.  Then allow link training by setting
PCIE_LTSSM_APP_DLY2_DONE

Co-developed-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
[bhelgaas: commit log]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://patch.msgid.link/20250613101908.2182053-2-cassel@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:

## 1. Fixes a Clear Race Condition Bug

The commit fixes a race condition between link training and DBI register
updates after a hot reset in EP mode. As stated in the commit message
and evidenced by the code changes:

- **Line 485-487** (original): When `PCIE_LINK_REQ_RST_NOT_INT`
  interrupt occurs, `dw_pcie_ep_linkdown()` is called which updates
  registers through DBI
- Without this fix, these DBI updates race with automatic link re-
  training after reset
- The RK3588 TRM explicitly documents this race condition and the proper
  mitigation

## 2. Small and Contained Fix

The changes are minimal and well-contained:
- Adds only 2 new bit definitions (`PCIE_LTSSM_APP_DLY2_EN` and
  `PCIE_LTSSM_APP_DLY2_DONE`)
- Modifies 3 locations with simple bit operations:
  - **Line 477**: Changes variable declaration to add `val`
  - **Lines 488-491**: Adds 3 lines to re-enable link training after DBI
    updates
  - **Lines 574-577**: Modifies initialization to enable automatic delay

## 3. Affects User-Visible Functionality

Without this fix, EP mode operation can experience:
- Corrupted register updates during hot reset scenarios
- Unpredictable behavior when the host performs hot reset
- Potential link training failures

## 4. Recent Feature with Active Bug Fixes

EP mode support was only added in commit e242f26f6320 (June 2024),
making this a relatively new feature that's still being stabilized. The
driver has seen multiple recent fixes:
- 286ed198b899: Fixed PHY function call sequence
- 7d9b5d611553: Fixed link up check
- 28b8d7793b85: Fixed PERST# GPIO value

## 5. Hardware-Documented Issue

This isn't a theoretical bug - it's explicitly documented in the RK3588
TRM section 11.6.1.3.3, providing clear hardware documentation that this
sequence is required for correct operation.

## 6. No Architectural Changes

The fix:
- Doesn't introduce new features
- Doesn't change APIs or interfaces
- Only ensures proper hardware sequencing per vendor documentation
- Has minimal risk of regression (only affects RK3588 EP mode operation)

The commit meets all criteria for stable backporting: it fixes a real
bug that affects users, is small and contained, doesn't introduce
features, and has minimal regression risk.

 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 93171a392879..cd1e9352b21f 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -58,6 +58,8 @@
 
 /* Hot Reset Control Register */
 #define PCIE_CLIENT_HOT_RESET_CTRL	0x180
+#define  PCIE_LTSSM_APP_DLY2_EN		BIT(1)
+#define  PCIE_LTSSM_APP_DLY2_DONE	BIT(3)
 #define  PCIE_LTSSM_ENABLE_ENHANCE	BIT(4)
 
 /* LTSSM Status Register */
@@ -474,7 +476,7 @@ static irqreturn_t rockchip_pcie_ep_sys_irq_thread(int irq, void *arg)
 	struct rockchip_pcie *rockchip = arg;
 	struct dw_pcie *pci = &rockchip->pci;
 	struct device *dev = pci->dev;
-	u32 reg;
+	u32 reg, val;
 
 	reg = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_INTR_STATUS_MISC);
 	rockchip_pcie_writel_apb(rockchip, reg, PCIE_CLIENT_INTR_STATUS_MISC);
@@ -485,6 +487,10 @@ static irqreturn_t rockchip_pcie_ep_sys_irq_thread(int irq, void *arg)
 	if (reg & PCIE_LINK_REQ_RST_NOT_INT) {
 		dev_dbg(dev, "hot reset or link-down reset\n");
 		dw_pcie_ep_linkdown(&pci->ep);
+		/* Stop delaying link training. */
+		val = HIWORD_UPDATE_BIT(PCIE_LTSSM_APP_DLY2_DONE);
+		rockchip_pcie_writel_apb(rockchip, val,
+					 PCIE_CLIENT_HOT_RESET_CTRL);
 	}
 
 	if (reg & PCIE_RDLH_LINK_UP_CHGED) {
@@ -566,8 +572,11 @@ static int rockchip_pcie_configure_ep(struct platform_device *pdev,
 		return ret;
 	}
 
-	/* LTSSM enable control mode */
-	val = HIWORD_UPDATE_BIT(PCIE_LTSSM_ENABLE_ENHANCE);
+	/*
+	 * LTSSM enable control mode, and automatically delay link training on
+	 * hot reset/link-down reset.
+	 */
+	val = HIWORD_UPDATE_BIT(PCIE_LTSSM_ENABLE_ENHANCE | PCIE_LTSSM_APP_DLY2_EN);
 	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
 
 	rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_EP_MODE,
-- 
2.39.5


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH AUTOSEL 6.16-6.6] phy: rockchip-pcie: Properly disable TEST_WRITE strobe signal

[Sasha Levin]

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.1751322015.git.geraldogabriel@gmail.com
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
-- 
2.39.5


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

[Sasha Levin]

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.1751322015.git.geraldogabriel@gmail.com
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


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip