[PATCH] phy: rockchip: snps-pcie3: Raise the SRAM init timeout

[PATCH] phy: rockchip: snps-pcie3: Raise the SRAM init timeout

[Shawn Lin]

The SRAM initialization of the PCIe3 PHY may take longer than the 500us
currently allowed, especially on a slow power-up such as at low
temperature. In that case the driver reports

  lock failed 0x..., check input refclk and power supply

even though the input refclk and power supply are fine.

Raise the timeout to 20ms and use a named constant for it, applied to
both the RK3568 and RK3588 paths.

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

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

diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
index 4e8ffd1..bbbfd6d 100644
--- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
+++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
@@ -19,6 +19,8 @@
 #include <linux/regmap.h>
 #include <linux/reset.h>
 
+#define RK_PCIE_SRAM_INIT_TIMEOUT		20000
+
 /* Register for RK3568 */
 #define GRF_PCIE30PHY_CON1			0x4
 #define GRF_PCIE30PHY_CON6			0x18
@@ -127,7 +129,7 @@ static int rockchip_p3phy_rk3568_init(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_PCIE_SRAM_INIT_TIMEOUT);
 	if (ret)
 		dev_err(&priv->phy->dev, "%s: lock failed 0x%x, check input refclk and power supply\n",
 		       __func__, reg);
@@ -187,11 +189,11 @@ static int rockchip_p3phy_rk3588_init(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_PCIE_SRAM_INIT_TIMEOUT);
 	ret |= regmap_read_poll_timeout(priv->phy_grf,
 					RK3588_PCIE3PHY_GRF_PHY1_STATUS1,
 					reg, RK3588_SRAM_INIT_DONE(reg),
-					0, 500);
+					0, RK_PCIE_SRAM_INIT_TIMEOUT);
 	if (ret)
 		dev_err(&priv->phy->dev, "lock failed 0x%x, check input refclk and power supply\n",
 			reg);
-- 
2.7.4


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH] phy: rockchip: snps-pcie3: Raise the SRAM init timeout

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] phy: rockchip: 20ms busy-wait loop in SRAM init polling stalls CPU
--

commit b3f51ba06ba11802539460b6b1bb17b8564adf62
Author: Shawn Lin <shawn.lin@rock-chips.com>
Date:   Thu Oct 15 07:02:55 2026 +0000

phy: rockchip: snps-pcie3: Raise the SRAM init timeout

This commit increases the SRAM initialization timeout for the PCIe3 PHY from
500us to 20ms. The longer timeout accommodates slower power-up sequences,
such as those at low temperatures, to prevent false lock failure reports.

> diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> index 4e8ffd173096a..bbbfd6d2e369b 100644
> --- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> +++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c

[ ... ]

> @@ -127,7 +129,7 @@ static int rockchip_p3phy_rk3568_init(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_PCIE_SRAM_INIT_TIMEOUT);

[Severity: Medium]
When the sleep_us parameter is 0, regmap_read_poll_timeout() uses cpu_relax()
and tight-loops instead of sleeping.

With the timeout increased from 500us to 20,000us (20ms), does this create an
excessively long busy-wait that could stall the CPU?

Since this initialization runs during driver probe or PHY power-on, which
happen under a mutex where sleeping is permitted, would it be better to provide
a non-zero sleep interval (like 1000) so the CPU can be yielded during this
extended 20ms wait?

>  	if (ret)
>  		dev_err(&priv->phy->dev, "%s: lock failed 0x%x, check input refclk and power supply\n",
>  		       __func__, reg);

[ ... ]

> @@ -187,11 +189,11 @@ static int rockchip_p3phy_rk3588_init(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_PCIE_SRAM_INIT_TIMEOUT);

[Severity: Medium]
Similarly here, for the RK3588 init path, does leaving the sleep interval at 0
mean we could spend up to 20ms busy-waiting?

>  	ret |= regmap_read_poll_timeout(priv->phy_grf,
>  					RK3588_PCIE3PHY_GRF_PHY1_STATUS1,
>  					reg, RK3588_SRAM_INIT_DONE(reg),
> -					0, 500);
> +					0, RK_PCIE_SRAM_INIT_TIMEOUT);

[Severity: Medium]
And since this second poll runs immediately after the first one, could we end
up busy-waiting for up to 40ms in total if both lanes are slow to lock?

>  	if (ret)
>  		dev_err(&priv->phy->dev, "lock failed 0x%x, check input refclk and power supply\n",
>  			reg);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/1792047775-77954-1-git-send-email-shawn.lin@rock-chips.com?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH] phy: rockchip: snps-pcie3: Raise the SRAM init timeout

[Shawn Lin]

在 2026/10/15 星期四 15:12, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] phy: rockchip: 20ms busy-wait loop in SRAM init polling stalls CPU
> --
> 

...

>> -				       0, 500);
>> +				       0, RK_PCIE_SRAM_INIT_TIMEOUT);
> 
> [Severity: Medium]
> When the sleep_us parameter is 0, regmap_read_poll_timeout() uses cpu_relax()
> and tight-loops instead of sleeping.
> 
> With the timeout increased from 500us to 20,000us (20ms), does this create an
> excessively long busy-wait that could stall the CPU?
> 
> Since this initialization runs during driver probe or PHY power-on, which
> happen under a mutex where sleeping is permitted, would it be better to provide
> a non-zero sleep interval (like 1000) so the CPU can be yielded during this
> extended 20ms wait?
> 

Good catch, will respin v2 to address it.

>>   	if (ret)
>>   		dev_err(&priv->phy->dev, "%s: lock failed 0x%x, check input refclk and power supply\n",

...

>>   	ret |= regmap_read_poll_timeout(priv->phy_grf,
>>   					RK3588_PCIE3PHY_GRF_PHY1_STATUS1,
>>   					reg, RK3588_SRAM_INIT_DONE(reg),
>> -					0, 500);
>> +					0, RK_PCIE_SRAM_INIT_TIMEOUT);
> 
> [Severity: Medium]
> And since this second poll runs immediately after the first one, could we end
> up busy-waiting for up to 40ms in total if both lanes are slow to lock?
> 

The two PHY lanes share most of their circuitry in both aggregation and
bifurcation mode, so in practice once the first PHY locks the second one
can be assumed to lock as well. The two sequential polls therefore do
not add any noticeable delay. I will add a comment into the commit
message for reference.


>>   	if (ret)
>>   		dev_err(&priv->phy->dev, "lock failed 0x%x, check input refclk and power supply\n",
>>   			reg);
> 


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy