[PATCH v1] phy: rockchip-snps-pcie3:phy: Configure clkreq_n and PowerDown for all lanes

[PATCH v1] phy: rockchip-snps-pcie3:phy: Configure clkreq_n and PowerDown for all lanes

[Anand Moon]

During the rk3588_p3phy_init sequence, the driver now explicitly
configures each lane's CON0 register to ensure
- PIPE 4.3 Compliance: clkreq_n (bit 6) is forced low (asserted) to meet
  sideband signal requirements.
- Active Power State: PowerDown[3:0] (bits 11:8) is set to P0
  (Normal Operational State) to ensure the PHY is fully powered and ready
  for link training.

These changes ensure that all lanes are consistently transitioned from
reset into a known-good operational state, preventing undefined behavior
and ensuring the PHY is ready for high-speed data transmission.

Cc: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 .../phy/rockchip/phy-rockchip-snps-pcie3.c    | 28 +++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
index 4e8ffd173096..f46e13e79a0e 100644
--- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
+++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
@@ -7,6 +7,7 @@
 
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/hw_bitfield.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
@@ -35,10 +36,14 @@
 #define RK3588_PCIE3PHY_GRF_CMN_CON0		0x0
 #define RK3588_PCIE3PHY_GRF_PHY0_STATUS1	0x904
 #define RK3588_PCIE3PHY_GRF_PHY1_STATUS1	0xa04
+#define RK3588_PCIE3PHY_GRF_PHY0_LN0_CON0	0x1000
 #define RK3588_PCIE3PHY_GRF_PHY0_LN0_CON1	0x1004
 #define RK3588_PCIE3PHY_GRF_PHY0_LN1_CON1	0x1104
+#define RK3588_PCIE3PHY_GRF_PHY0_LN1_CON0	0x1100
+#define RK3588_PCIE3PHY_GRF_PHY1_LN0_CON0	0x2000
 #define RK3588_PCIE3PHY_GRF_PHY1_LN0_CON1	0x2004
 #define RK3588_PCIE3PHY_GRF_PHY1_LN1_CON1	0x2104
+#define RK3588_PCIE3PHY_GRF_PHY1_LN1_CON0	0x2100
 #define RK3588_SRAM_INIT_DONE(reg)		(reg & BIT(0))
 
 #define RK3588_BIFURCATION_LANE_0_1		BIT(0)
@@ -49,6 +54,13 @@
 #define RK3588_PCIE1LN_SEL_EN			(GENMASK(1, 0) << 16)
 #define RK3588_PCIE30_PHY_MODE_EN		(GENMASK(2, 0) << 16)
 
+static const u32 rk3588_lane_con0[] = {
+	RK3588_PCIE3PHY_GRF_PHY0_LN0_CON0,
+	RK3588_PCIE3PHY_GRF_PHY0_LN1_CON0,
+	RK3588_PCIE3PHY_GRF_PHY1_LN0_CON0,
+	RK3588_PCIE3PHY_GRF_PHY1_LN1_CON0,
+};
+
 struct rockchip_p3phy_ops;
 
 struct rockchip_p3phy_priv {
@@ -142,7 +154,7 @@ static int rockchip_p3phy_rk3588_init(struct rockchip_p3phy_priv *priv)
 {
 	u32 reg = 0;
 	u8 mode = RK3588_LANE_AGGREGATION; /* default */
-	int ret;
+	int ret, i;
 
 	regmap_write(priv->phy_grf, RK3588_PCIE3PHY_GRF_PHY0_LN0_CON1,
 		     priv->rx_cmn_refclk_mode[0] ? RK3588_RX_CMN_REFCLK_MODE_EN :
@@ -161,7 +173,7 @@ static int rockchip_p3phy_rk3588_init(struct rockchip_p3phy_priv *priv)
 	regmap_write(priv->phy_grf, RK3588_PCIE3PHY_GRF_CMN_CON0, BIT(8) | BIT(24));
 
 	/* Set bifurcation if needed */
-	for (int i = 0; i < priv->num_lanes; i++) {
+	for (i = 0; i < priv->num_lanes; i++) {
 		if (priv->lanes[i] > 1)
 			mode &= ~RK3588_LANE_AGGREGATION;
 		if (priv->lanes[i] == 3)
@@ -174,6 +186,18 @@ static int rockchip_p3phy_rk3588_init(struct rockchip_p3phy_priv *priv)
 	regmap_write(priv->phy_grf, RK3588_PCIE3PHY_GRF_CMN_CON0,
 		     RK3588_PCIE30_PHY_MODE_EN | reg);
 
+	for (i = 0; i < priv->num_lanes && i < ARRAY_SIZE(rk3588_lane_con0); i++) {
+		u32 base = rk3588_lane_con0[i];
+
+		/* clkreq_n = 0 (asserted low for PIPE 4.3) */
+		regmap_write(priv->phy_grf, base,
+			     FIELD_PREP_WM16(BIT(6), 0));
+
+		/* PowerDown = P0 (0x0, fully active) */
+		regmap_write(priv->phy_grf, base,
+			     FIELD_PREP_WM16(GENMASK(11, 8), 0x0));
+	}
+
 	/* Set pcie1ln_sel in PHP_GRF_PCIESEL_CON */
 	if (!IS_ERR(priv->pipe_grf)) {
 		reg = mode & (RK3588_BIFURCATION_LANE_0_1 | RK3588_BIFURCATION_LANE_2_3);

base-commit: 7f87a5ea75f011d2c9bc8ac0167e5e2d1adb1594
-- 
2.50.1

Re: [PATCH v1] phy: rockchip-snps-pcie3:phy: Configure clkreq_n and PowerDown for all lanes

[Niklas Cassel]

+Shawn

Hello Anand,

On Thu, Apr 09, 2026 at 10:19:30AM +0530, Anand Moon wrote:
> During the rk3588_p3phy_init sequence, the driver now explicitly

Please use imperative mood, active voice.


> configures each lane's CON0 register to ensure
> - PIPE 4.3 Compliance: clkreq_n (bit 6) is forced low (asserted) to meet
>   sideband signal requirements.
> - Active Power State: PowerDown[3:0] (bits 11:8) is set to P0
>   (Normal Operational State) to ensure the PHY is fully powered and ready
>   for link training.
> 
> These changes ensure that all lanes are consistently transitioned from
> reset into a known-good operational state, preventing undefined behavior
> and ensuring the PHY is ready for high-speed data transmission.

First describe the problem, then describe how you fix it.


Kind regards,
Niklas

Re: [PATCH v1] phy: rockchip-snps-pcie3:phy: Configure clkreq_n and PowerDown for all lanes

[Anand Moon]

Hi Niklas,

Thanks for your review comments.

On Thu, 9 Apr 2026 at 15:19, Niklas Cassel <cassel@kernel.org> wrote:
>
> +Shawn
>
> Hello Anand,
>
> On Thu, Apr 09, 2026 at 10:19:30AM +0530, Anand Moon wrote:
> > During the rk3588_p3phy_init sequence, the driver now explicitly
>
> Please use imperative mood, active voice.
>
>
> > configures each lane's CON0 register to ensure
> > - PIPE 4.3 Compliance: clkreq_n (bit 6) is forced low (asserted) to meet
> >   sideband signal requirements.
> > - Active Power State: PowerDown[3:0] (bits 11:8) is set to P0
> >   (Normal Operational State) to ensure the PHY is fully powered and ready
> >   for link training.
> >
> > These changes ensure that all lanes are consistently transitioned from
> > reset into a known-good operational state, preventing undefined behavior
> > and ensuring the PHY is ready for high-speed data transmission.
>
Ok, I will update this.I f
> First describe the problem, then describe how you fix it.
I was investigating the PCIE30X4_CLKREQn issue highlighted by Shawn Lin,
analyzing the RK3588 TRM clock request configurations (page 878)

[1] https://lore.kernel.org/all/77f0d3c2-649f-770d-1636-6fd52f3b5f5e@rock-chips.com/

Looking into the power management state on Intel’s PCI Express Power Management
documentation, which defines states such as P0, P0s, P1, P1.1, P1.2, and P2
Understanding this mapping is for interpreting the behavior of lane-specific
clock requests, so initialize this to the P0 state..

[2] https://www.intel.com/content/dam/www/public/us/en/documents/white-papers/phy-interface-pci-express-sata-usb30-architectures-3.1.pdf
>
>
> Kind regards,
> Niklas

Thanks
-Anand