[PATCH] phy: rockchip: snps-pcie3: Raise the SRAM init timeout
Shawn Lin
shawn.lin at rock-chips.com
Thu Oct 15 01:04:28 PDT 2026
在 2026/10/15 星期四 15:12, sashiko-bot at 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);
>
More information about the linux-phy
mailing list