[PATCH 0/5] Add calibration for Synopsys PCIe PHY and Controller

[PATCH 0/5] Add calibration for Synopsys PCIe PHY and Controller

[Shawn Lin]

Currently, when pcie-dw-rockchip uses the Synopsys PHY, it relies on
the phy_init() callback of the phy-rockchip-snps-pcie3 driver to
perform calibration. This is incorrect because the controller is
still held in reset at that time, preventing the PHY from accurately
reflecting the actual PLL lock and calibration status.

To fix this, this series:
1. Calls phy_calibrate() in the pcie-dw-rockchip driver (if supported)
   after the controller is out of reset, ensuring the PHY can
   properly synchronize with the controller state.
2. Adds the necessary calibration support in the Synopsys PHY driver
   to implement this callback.

Please review and test.



Shawn Lin (5):
  PCI: dw-rockchip: Add phy_calibrate() to check PHY lock status
  phy: rockchip-snps-pcie3: Add phy_calibrate() support
  phy: rockchip-snps-pcie3: Increase sram init timeout
  phy: rockchip-snps-pcie3: Check more sram init status for RK3588
  phy: rockchip-snps-pcie3: Only check PHY1 status when using it

 drivers/pci/controller/dwc/pcie-dw-rockchip.c  |  9 +++-
 drivers/phy/rockchip/phy-rockchip-snps-pcie3.c | 61 +++++++++++++++++++++-----
 2 files changed, 57 insertions(+), 13 deletions(-)

-- 
2.7.4


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

[PATCH 1/5] PCI: dw-rockchip: Add phy_calibrate() to check PHY lock status

[Shawn Lin]

Current we keep controller in reset state when initializing PHY which
is the right thing to do. But this case, the PHY is also reset because
it refers to a signal from controller. Now we check PHY lock status
inside .phy_init() callback which may be bogus for certain type of PHY,
because of the fact above. Add phy_calibrate() to better check PHY lock
status if provided.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index f8605fe..75d6306 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -705,6 +705,12 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		goto deinit_phy;
 
+	ret = phy_calibrate(rockchip->phy);
+	if (ret) {
+		dev_err(dev, "phy lock failed\n");
+		goto assert_controller;
+	}
+
 	ret = rockchip_pcie_clk_init(rockchip);
 	if (ret)
 		goto deinit_phy;
@@ -727,7 +733,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 	}
 
 	return 0;
-
+assert_controller:
+	reset_control_assert(rockchip->rst);
 deinit_clk:
 	clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
 deinit_phy:
-- 
2.7.4


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

[PATCH 2/5] phy: rockchip-snps-pcie3: Add phy_calibrate() support

[Shawn Lin]

Move calibration from phy_init() to phy_calibrate().

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/phy/rockchip/phy-rockchip-snps-pcie3.c | 39 ++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
index 4e8ffd1..9933cda 100644
--- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
+++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
@@ -71,6 +71,7 @@ struct rockchip_p3phy_priv {
 
 struct rockchip_p3phy_ops {
 	int (*phy_init)(struct rockchip_p3phy_priv *priv);
+	int (*phy_calibrate)(struct rockchip_p3phy_priv *priv);
 };
 
 static int rockchip_p3phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
@@ -97,8 +98,6 @@ static int rockchip_p3phy_rk3568_init(struct rockchip_p3phy_priv *priv)
 {
 	struct phy *phy = priv->phy;
 	bool bifurcation = false;
-	int ret;
-	u32 reg;
 
 	/* Deassert PCIe PMA output clamp mode */
 	regmap_write(priv->phy_grf, GRF_PCIE30PHY_CON9, GRF_PCIE30PHY_DA_OCM);
@@ -124,25 +123,34 @@ static int rockchip_p3phy_rk3568_init(struct rockchip_p3phy_priv *priv)
 
 	reset_control_deassert(priv->p30phy);
 
+	return 0;
+}
+
+static int rockchip_p3phy_rk3568_calibrate(struct rockchip_p3phy_priv *priv)
+{
+	int ret;
+	u32 reg;
+
 	ret = regmap_read_poll_timeout(priv->phy_grf,
 				       GRF_PCIE30PHY_STATUS0,
 				       reg, SRAM_INIT_DONE(reg),
 				       0, 500);
 	if (ret)
-		dev_err(&priv->phy->dev, "%s: lock failed 0x%x, check input refclk and power supply\n",
-		       __func__, reg);
+		dev_err(&priv->phy->dev, "lock failed 0x%x, check input refclk and power supply\n",
+			reg);
+
 	return ret;
 }
 
 static const struct rockchip_p3phy_ops rk3568_ops = {
 	.phy_init = rockchip_p3phy_rk3568_init,
+	.phy_calibrate = rockchip_p3phy_rk3568_calibrate,
 };
 
 static int rockchip_p3phy_rk3588_init(struct rockchip_p3phy_priv *priv)
 {
 	u32 reg = 0;
 	u8 mode = RK3588_LANE_AGGREGATION; /* default */
-	int ret;
 
 	regmap_write(priv->phy_grf, RK3588_PCIE3PHY_GRF_PHY0_LN0_CON1,
 		     priv->rx_cmn_refclk_mode[0] ? RK3588_RX_CMN_REFCLK_MODE_EN :
@@ -184,6 +192,14 @@ static int rockchip_p3phy_rk3588_init(struct rockchip_p3phy_priv *priv)
 
 	reset_control_deassert(priv->p30phy);
 
+	return 0;
+}
+
+static int rockchip_p3phy_rk3588_calibrate(struct rockchip_p3phy_priv *priv)
+{
+	int ret;
+	u32 reg;
+
 	ret = regmap_read_poll_timeout(priv->phy_grf,
 				       RK3588_PCIE3PHY_GRF_PHY0_STATUS1,
 				       reg, RK3588_SRAM_INIT_DONE(reg),
@@ -200,6 +216,7 @@ static int rockchip_p3phy_rk3588_init(struct rockchip_p3phy_priv *priv)
 
 static const struct rockchip_p3phy_ops rk3588_ops = {
 	.phy_init = rockchip_p3phy_rk3588_init,
+	.phy_calibrate = rockchip_p3phy_rk3588_calibrate,
 };
 
 static int rockchip_p3phy_init(struct phy *phy)
@@ -234,10 +251,22 @@ static int rockchip_p3phy_exit(struct phy *phy)
 	return 0;
 }
 
+static int rockchip_p3phy_calibrate(struct phy *phy)
+{
+	struct rockchip_p3phy_priv *priv = phy_get_drvdata(phy);
+	int ret = 0;
+
+	if (priv->ops->phy_calibrate)
+		ret = priv->ops->phy_calibrate(priv);
+
+	return ret;
+}
+
 static const struct phy_ops rockchip_p3phy_ops = {
 	.init = rockchip_p3phy_init,
 	.exit = rockchip_p3phy_exit,
 	.set_mode = rockchip_p3phy_set_mode,
+	.calibrate = rockchip_p3phy_calibrate,
 	.owner = THIS_MODULE,
 };
 
-- 
2.7.4


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

[PATCH 3/5] phy: rockchip-snps-pcie3: Increase sram init timeout

[Shawn Lin]

Per massive test, 500us is not enough for all chips, increase it
to 20000us for worse case recommended.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/phy/rockchip/phy-rockchip-snps-pcie3.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
index 9933cda..f5a5d0af 100644
--- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
+++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
@@ -19,6 +19,9 @@
 #include <linux/regmap.h>
 #include <linux/reset.h>
 
+/* Common definition */
+#define RK_SRAM_INIT_TIMEOUT_US			20000
+
 /* Register for RK3568 */
 #define GRF_PCIE30PHY_CON1			0x4
 #define GRF_PCIE30PHY_CON6			0x18
@@ -28,6 +31,7 @@
 #define GRF_PCIE30PHY_WR_EN			(0xf << 16)
 #define SRAM_INIT_DONE(reg)			(reg & BIT(14))
 
+
 #define RK3568_BIFURCATION_LANE_0_1		BIT(0)
 
 /* Register for RK3588 */
@@ -134,7 +138,7 @@ static int rockchip_p3phy_rk3568_calibrate(struct rockchip_p3phy_priv *priv)
 	ret = regmap_read_poll_timeout(priv->phy_grf,
 				       GRF_PCIE30PHY_STATUS0,
 				       reg, SRAM_INIT_DONE(reg),
-				       0, 500);
+				       0, RK_SRAM_INIT_TIMEOUT_US);
 	if (ret)
 		dev_err(&priv->phy->dev, "lock failed 0x%x, check input refclk and power supply\n",
 			reg);
@@ -203,11 +207,11 @@ static int rockchip_p3phy_rk3588_calibrate(struct rockchip_p3phy_priv *priv)
 	ret = regmap_read_poll_timeout(priv->phy_grf,
 				       RK3588_PCIE3PHY_GRF_PHY0_STATUS1,
 				       reg, RK3588_SRAM_INIT_DONE(reg),
-				       0, 500);
+				       0, RK_SRAM_INIT_TIMEOUT_US);
 	ret |= regmap_read_poll_timeout(priv->phy_grf,
 					RK3588_PCIE3PHY_GRF_PHY1_STATUS1,
 					reg, RK3588_SRAM_INIT_DONE(reg),
-					0, 500);
+					0, RK_SRAM_INIT_TIMEOUT_US);
 	if (ret)
 		dev_err(&priv->phy->dev, "lock failed 0x%x, check input refclk and power supply\n",
 			reg);
-- 
2.7.4


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

[PATCH 4/5] phy: rockchip-snps-pcie3: Check more sram init status for RK3588

[Shawn Lin]

All the lower 4 bits should be checked which shows the mpllx_state.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/phy/rockchip/phy-rockchip-snps-pcie3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
index f5a5d0af..6cc38e3 100644
--- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
+++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
@@ -43,7 +43,7 @@
 #define RK3588_PCIE3PHY_GRF_PHY0_LN1_CON1	0x1104
 #define RK3588_PCIE3PHY_GRF_PHY1_LN0_CON1	0x2004
 #define RK3588_PCIE3PHY_GRF_PHY1_LN1_CON1	0x2104
-#define RK3588_SRAM_INIT_DONE(reg)		(reg & BIT(0))
+#define RK3588_SRAM_INIT_DONE(reg)              ((reg & 0xf) == 0xf)
 
 #define RK3588_BIFURCATION_LANE_0_1		BIT(0)
 #define RK3588_BIFURCATION_LANE_2_3		BIT(1)
-- 
2.7.4


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

[PATCH 5/5] phy: rockchip-snps-pcie3: Only check PHY1 status when using it

[Shawn Lin]

RK3588_LANE_AGGREGATION and RK3588_BIFURCATION_LANE_2_3 should be
used to check if it need to check PHY1 status. Because in other
cases, only PHY0 could show locked status.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/phy/rockchip/phy-rockchip-snps-pcie3.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
index 6cc38e3..36b2142 100644
--- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
+++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
@@ -183,6 +183,7 @@ static int rockchip_p3phy_rk3588_init(struct rockchip_p3phy_priv *priv)
 	}
 
 	reg = mode;
+	priv->pcie30_phymode = mode;
 	regmap_write(priv->phy_grf, RK3588_PCIE3PHY_GRF_CMN_CON0,
 		     RK3588_PCIE30_PHY_MODE_EN | reg);
 
@@ -208,10 +209,13 @@ static int rockchip_p3phy_rk3588_calibrate(struct rockchip_p3phy_priv *priv)
 				       RK3588_PCIE3PHY_GRF_PHY0_STATUS1,
 				       reg, RK3588_SRAM_INIT_DONE(reg),
 				       0, RK_SRAM_INIT_TIMEOUT_US);
-	ret |= regmap_read_poll_timeout(priv->phy_grf,
-					RK3588_PCIE3PHY_GRF_PHY1_STATUS1,
-					reg, RK3588_SRAM_INIT_DONE(reg),
-					0, RK_SRAM_INIT_TIMEOUT_US);
+	if (priv->pcie30_phymode & (RK3588_LANE_AGGREGATION | RK3588_BIFURCATION_LANE_2_3)) {
+		ret |= regmap_read_poll_timeout(priv->phy_grf,
+						RK3588_PCIE3PHY_GRF_PHY1_STATUS1,
+						reg, RK3588_SRAM_INIT_DONE(reg),
+						0, RK_SRAM_INIT_TIMEOUT_US);
+	}
+
 	if (ret)
 		dev_err(&priv->phy->dev, "lock failed 0x%x, check input refclk and power supply\n",
 			reg);
-- 
2.7.4


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

Re: [PATCH 1/5] PCI: dw-rockchip: Add phy_calibrate() to check PHY lock status

[Manivannan Sadhasivam]

On Wed, Dec 24, 2025 at 03:10:06PM +0800, Shawn Lin wrote:
> Current we keep controller in reset state when initializing PHY which
> is the right thing to do. But this case, the PHY is also reset because
> it refers to a signal from controller. Now we check PHY lock status
> inside .phy_init() callback which may be bogus for certain type of PHY,
> because of the fact above. Add phy_calibrate() to better check PHY lock
> status if provided.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

I guess this patch can get merged separately without the PHY patches and not
cause any functional issue. But I'd like the PHY patches to get reviewed so that
we know that the API usage is correct.

- Mani

> ---
> 
>  drivers/pci/controller/dwc/pcie-dw-rockchip.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index f8605fe..75d6306 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -705,6 +705,12 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto deinit_phy;
>  
> +	ret = phy_calibrate(rockchip->phy);
> +	if (ret) {
> +		dev_err(dev, "phy lock failed\n");
> +		goto assert_controller;
> +	}
> +
>  	ret = rockchip_pcie_clk_init(rockchip);
>  	if (ret)
>  		goto deinit_phy;
> @@ -727,7 +733,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	}
>  
>  	return 0;
> -
> +assert_controller:
> +	reset_control_assert(rockchip->rst);
>  deinit_clk:
>  	clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
>  deinit_phy:
> -- 
> 2.7.4
> 

-- 
மணிவண்ணன் சதாசிவம்

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

Re: [PATCH 3/5] phy: rockchip-snps-pcie3: Increase sram init timeout

[Vinod Koul]

On 24-12-25, 15:10, Shawn Lin wrote:
> Per massive test, 500us is not enough for all chips, increase it
> to 20000us for worse case recommended.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
>  drivers/phy/rockchip/phy-rockchip-snps-pcie3.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> index 9933cda..f5a5d0af 100644
> --- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> +++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> @@ -19,6 +19,9 @@
>  #include <linux/regmap.h>
>  #include <linux/reset.h>
>  
> +/* Common definition */
> +#define RK_SRAM_INIT_TIMEOUT_US			20000
> +
>  /* Register for RK3568 */
>  #define GRF_PCIE30PHY_CON1			0x4
>  #define GRF_PCIE30PHY_CON6			0x18
> @@ -28,6 +31,7 @@
>  #define GRF_PCIE30PHY_WR_EN			(0xf << 16)
>  #define SRAM_INIT_DONE(reg)			(reg & BIT(14))
>  
> +

why this empty line here?

>  #define RK3568_BIFURCATION_LANE_0_1		BIT(0)
>  
>  /* Register for RK3588 */
> @@ -134,7 +138,7 @@ static int rockchip_p3phy_rk3568_calibrate(struct rockchip_p3phy_priv *priv)
>  	ret = regmap_read_poll_timeout(priv->phy_grf,
>  				       GRF_PCIE30PHY_STATUS0,
>  				       reg, SRAM_INIT_DONE(reg),
> -				       0, 500);
> +				       0, RK_SRAM_INIT_TIMEOUT_US);
>  	if (ret)
>  		dev_err(&priv->phy->dev, "lock failed 0x%x, check input refclk and power supply\n",
>  			reg);
> @@ -203,11 +207,11 @@ static int rockchip_p3phy_rk3588_calibrate(struct rockchip_p3phy_priv *priv)
>  	ret = regmap_read_poll_timeout(priv->phy_grf,
>  				       RK3588_PCIE3PHY_GRF_PHY0_STATUS1,
>  				       reg, RK3588_SRAM_INIT_DONE(reg),
> -				       0, 500);
> +				       0, RK_SRAM_INIT_TIMEOUT_US);
>  	ret |= regmap_read_poll_timeout(priv->phy_grf,
>  					RK3588_PCIE3PHY_GRF_PHY1_STATUS1,
>  					reg, RK3588_SRAM_INIT_DONE(reg),
> -					0, 500);
> +					0, RK_SRAM_INIT_TIMEOUT_US);
>  	if (ret)
>  		dev_err(&priv->phy->dev, "lock failed 0x%x, check input refclk and power supply\n",
>  			reg);
> -- 
> 2.7.4

-- 
~Vinod

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

Re: [PATCH 0/5] Add calibration for Synopsys PCIe PHY and Controller

[Niklas Cassel]

On Wed, Dec 24, 2025 at 03:10:05PM +0800, Shawn Lin wrote:
> 
> Currently, when pcie-dw-rockchip uses the Synopsys PHY, it relies on
> the phy_init() callback of the phy-rockchip-snps-pcie3 driver to
> perform calibration. This is incorrect because the controller is
> still held in reset at that time, preventing the PHY from accurately
> reflecting the actual PLL lock and calibration status.

Hello Shawn,

I can see that you move the calibration code from .phy_init() to
.phy_calibrate().

And I understandthat the controller is still held in reset.

I understand that the the PHY calibration is supposed to be done
when the controller is not held in reset, and that alone is
enough to warrant a fix.

The Synopsys Gen3 PHY is used in e.g. Rock5b, and link training
currently works fine with this PHY, so what is the actual
implications of performing the PHY calibration when the controller
is held in reset?

Will it somehow it improve signal integrity?


Kind regards,
Niklas

> 
> To fix this, this series:
> 1. Calls phy_calibrate() in the pcie-dw-rockchip driver (if supported)
>    after the controller is out of reset, ensuring the PHY can
>    properly synchronize with the controller state.
> 2. Adds the necessary calibration support in the Synopsys PHY driver
>    to implement this callback.

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

Re: [PATCH 3/5] phy: rockchip-snps-pcie3: Increase sram init timeout

[Shawn Lin]

在 2026/01/14 星期三 21:29, Vinod Koul 写道:
> On 24-12-25, 15:10, Shawn Lin wrote:
>> Per massive test, 500us is not enough for all chips, increase it
>> to 20000us for worse case recommended.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>>   drivers/phy/rockchip/phy-rockchip-snps-pcie3.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
>> index 9933cda..f5a5d0af 100644
>> --- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
>> +++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
>> @@ -19,6 +19,9 @@
>>   #include <linux/regmap.h>
>>   #include <linux/reset.h>
>>   
>> +/* Common definition */
>> +#define RK_SRAM_INIT_TIMEOUT_US			20000
>> +
>>   /* Register for RK3568 */
>>   #define GRF_PCIE30PHY_CON1			0x4
>>   #define GRF_PCIE30PHY_CON6			0x18
>> @@ -28,6 +31,7 @@
>>   #define GRF_PCIE30PHY_WR_EN			(0xf << 16)
>>   #define SRAM_INIT_DONE(reg)			(reg & BIT(14))
>>   
>> +
> 
> why this empty line here?
> 

Oops, will fix it.

>>   #define RK3568_BIFURCATION_LANE_0_1		BIT(0)
>>   
>>   /* Register for RK3588 */
>> @@ -134,7 +138,7 @@ static int rockchip_p3phy_rk3568_calibrate(struct rockchip_p3phy_priv *priv)
>>   	ret = regmap_read_poll_timeout(priv->phy_grf,
>>   				       GRF_PCIE30PHY_STATUS0,
>>   				       reg, SRAM_INIT_DONE(reg),
>> -				       0, 500);
>> +				       0, RK_SRAM_INIT_TIMEOUT_US);
>>   	if (ret)
>>   		dev_err(&priv->phy->dev, "lock failed 0x%x, check input refclk and power supply\n",
>>   			reg);
>> @@ -203,11 +207,11 @@ static int rockchip_p3phy_rk3588_calibrate(struct rockchip_p3phy_priv *priv)
>>   	ret = regmap_read_poll_timeout(priv->phy_grf,
>>   				       RK3588_PCIE3PHY_GRF_PHY0_STATUS1,
>>   				       reg, RK3588_SRAM_INIT_DONE(reg),
>> -				       0, 500);
>> +				       0, RK_SRAM_INIT_TIMEOUT_US);
>>   	ret |= regmap_read_poll_timeout(priv->phy_grf,
>>   					RK3588_PCIE3PHY_GRF_PHY1_STATUS1,
>>   					reg, RK3588_SRAM_INIT_DONE(reg),
>> -					0, 500);
>> +					0, RK_SRAM_INIT_TIMEOUT_US);
>>   	if (ret)
>>   		dev_err(&priv->phy->dev, "lock failed 0x%x, check input refclk and power supply\n",
>>   			reg);
>> -- 
>> 2.7.4
> 


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

Re: [PATCH 0/5] Add calibration for Synopsys PCIe PHY and Controller

[Shawn Lin]

在 2026/01/14 星期三 23:43, Niklas Cassel 写道:
> On Wed, Dec 24, 2025 at 03:10:05PM +0800, Shawn Lin wrote:
>>
>> Currently, when pcie-dw-rockchip uses the Synopsys PHY, it relies on
>> the phy_init() callback of the phy-rockchip-snps-pcie3 driver to
>> perform calibration. This is incorrect because the controller is
>> still held in reset at that time, preventing the PHY from accurately
>> reflecting the actual PLL lock and calibration status.
> 
> Hello Shawn,
> 
> I can see that you move the calibration code from .phy_init() to
> .phy_calibrate().
> 
> And I understandthat the controller is still held in reset.
> 
> I understand that the the PHY calibration is supposed to be done
> when the controller is not held in reset, and that alone is
> enough to warrant a fix.

Sure.

> 
> The Synopsys Gen3 PHY is used in e.g. Rock5b, and link training
> currently works fine with this PHY, so what is the actual

It just happended to work as in most cases, the calibration finished
very quickly after controller is not held in reset.

> implications of performing the PHY calibration when the controller
> is held in reset?
> 
> Will it somehow it improve signal integrity?
> 

Performing the PHY calibration when the controller is held
in reset is the wrong way. If the refclk or PHY power supply isn't
ready, the bogus calibration still passes, then the system will get
stuck when accessing DBI. So performing the PHY calibration must be done
after controller quit the reset state.

> 
> Kind regards,
> Niklas
> 
>>
>> To fix this, this series:
>> 1. Calls phy_calibrate() in the pcie-dw-rockchip driver (if supported)
>>     after the controller is out of reset, ensuring the PHY can
>>     properly synchronize with the controller state.
>> 2. Adds the necessary calibration support in the Synopsys PHY driver
>>     to implement this callback.
> 


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

Re: [PATCH 4/5] phy: rockchip-snps-pcie3: Check more sram init status for RK3588

[Alexey Charkov]

Hi Shawn,

On Wed, Dec 24, 2025 at 11:10 AM Shawn Lin <shawn.lin@rock-chips.com> wrote:
>
> All the lower 4 bits should be checked which shows the mpllx_state.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
>
>  drivers/phy/rockchip/phy-rockchip-snps-pcie3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> index f5a5d0af..6cc38e3 100644
> --- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> +++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> @@ -43,7 +43,7 @@
>  #define RK3588_PCIE3PHY_GRF_PHY0_LN1_CON1      0x1104
>  #define RK3588_PCIE3PHY_GRF_PHY1_LN0_CON1      0x2004
>  #define RK3588_PCIE3PHY_GRF_PHY1_LN1_CON1      0x2104
> -#define RK3588_SRAM_INIT_DONE(reg)             (reg & BIT(0))
> +#define RK3588_SRAM_INIT_DONE(reg)              ((reg & 0xf) == 0xf)

This breaks PCIe3x4 on my Radxa ROCK 5B, which then cannot find its
NVMe root filesystem. Sebastian's rockchip-devel branch carries this
series, and kernels built from it fail with:

  phy phy-fee80000.phy.13: lock failed 0x7, check input refclk and power supply
  rockchip-dw-pcie a40000000.pcie: phy lock failed
  rockchip-dw-pcie a40000000.pcie: probe with driver rockchip-dw-pcie
failed with error -110

The RK3588 TRM (V1.0, Part 1) describes PCIe3PHY_GRF_PHY{0,1}_STATUS1
as follows:

  bit 0     sram_init_done
  bit 1     pcs_pwr_en     (reset value 1)
  bit 2     pma_pwr_en     (reset value 1)
  bit 3     mplla_state
  bits 5:4  reserved
  bit 6     mpllb_state

So the lower four bits are not all PLL states: bits 1 and 2 are power
switch enables, and the only PLL among them is MPLLA, while MPLLB is
bit 6.

On this board MPLLA is off even on a working link. With this patch
reverted, PHY0_STATUS1 and PHY1_STATUS1 both read 0x47 once the link
has trained at 8 GT/s x4: SRAM initialised, MPLLB running, MPLLA off.
When the check fails they read 0x7, so neither PLL is running yet at
that point, and checking for MPLLB instead would fail too.

Reverting only this patch, with the rest of the series applied
(rockchip-devel at d7436346fc31, v7.4-rc2), brings PCIe3x4 and the
NVMe back.

What is the check meant to catch? If it is PLL lock, which bits, and
at what point, would be right for a link that runs on MPLLB?
Otherwise, sram_init_done (bit 0) alone looks like the right condition
here.

Best regards,
Alexey

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

Re: [PATCH 4/5] phy: rockchip-snps-pcie3: Check more sram init status for RK3588

[Shawn Lin]

Hi Alexey

在 2026/11/13 星期五 16:46, Alexey Charkov 写道:
> Hi Shawn,
> 
> On Wed, Dec 24, 2025 at 11:10 AM Shawn Lin <shawn.lin@rock-chips.com> wrote:
>>
>> All the lower 4 bits should be checked which shows the mpllx_state.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>>   drivers/phy/rockchip/phy-rockchip-snps-pcie3.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
>> index f5a5d0af..6cc38e3 100644
>> --- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
>> +++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
>> @@ -43,7 +43,7 @@
>>   #define RK3588_PCIE3PHY_GRF_PHY0_LN1_CON1      0x1104
>>   #define RK3588_PCIE3PHY_GRF_PHY1_LN0_CON1      0x2004
>>   #define RK3588_PCIE3PHY_GRF_PHY1_LN1_CON1      0x2104
>> -#define RK3588_SRAM_INIT_DONE(reg)             (reg & BIT(0))
>> +#define RK3588_SRAM_INIT_DONE(reg)              ((reg & 0xf) == 0xf)
> 
> This breaks PCIe3x4 on my Radxa ROCK 5B, which then cannot find its
> NVMe root filesystem. Sebastian's rockchip-devel branch carries this
> series, and kernels built from it fail with:
> 
>    phy phy-fee80000.phy.13: lock failed 0x7, check input refclk and power supply
>    rockchip-dw-pcie a40000000.pcie: phy lock failed
>    rockchip-dw-pcie a40000000.pcie: probe with driver rockchip-dw-pcie
> failed with error -110
> 
> The RK3588 TRM (V1.0, Part 1) describes PCIe3PHY_GRF_PHY{0,1}_STATUS1
> as follows:
> 
>    bit 0     sram_init_done
>    bit 1     pcs_pwr_en     (reset value 1)
>    bit 2     pma_pwr_en     (reset value 1)
>    bit 3     mplla_state
>    bits 5:4  reserved
>    bit 6     mpllb_state
> 
> So the lower four bits are not all PLL states: bits 1 and 2 are power
> switch enables, and the only PLL among them is MPLLA, while MPLLB is
> bit 6.
> 
> On this board MPLLA is off even on a working link. With this patch
> reverted, PHY0_STATUS1 and PHY1_STATUS1 both read 0x47 once the link
> has trained at 8 GT/s x4: SRAM initialised, MPLLB running, MPLLA off.
> When the check fails they read 0x7, so neither PLL is running yet at
> that point, and checking for MPLLB instead would fail too.
> 
> Reverting only this patch, with the rest of the series applied
> (rockchip-devel at d7436346fc31, v7.4-rc2), brings PCIe3x4 and the
> NVMe back.
> 

Each port has a MPLLA + a MPLLB. In aggregation mode, MPLLA should
always be 1 once done for both of port0 and port1. But in bifurcation
mode, MPLLB will change. So basiclly, Rockchip PHY team suggests to
check mplla instead of mpllb for differnt use cases, during calibration.
That was done for vendor kernel for quite a long time. So I was
surprising to know in your case, rock 5b using it in aggregation mode,
it didn't work that way. Please let me ask interanlly...

> What is the check meant to catch? If it is PLL lock, which bits, and
> at what point, would be right for a link that runs on MPLLB?
> Otherwise, sram_init_done (bit 0) alone looks like the right condition
> here.
> 
> Best regards,
> Alexey
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


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