Linux-PHY Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] phy: starfive: jh7110-dphy: fail init when the PLL does not lock
@ 2026-06-24 14:39 Pengpeng Hou
  2026-06-24 14:48 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Pengpeng Hou @ 2026-06-24 14:39 UTC (permalink / raw)
  To: Keith Zhao, Vinod Koul, Neil Armstrong
  Cc: Pengpeng Hou, linux-phy, linux-kernel

stf_dphy_hw_reset() polls the DPHY PLL unlock bit when bringing the PHY
out of reset, but only logs a timeout. stf_dphy_init() then continues
and returns success even though the PHY lock condition was not observed.

Return the poll error from the reset helper and propagate it from the PHY
init path. Also make the timeout message describe the failed lock
condition.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/phy/starfive/phy-jh7110-dphy-tx.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/starfive/phy-jh7110-dphy-tx.c b/drivers/phy/starfive/phy-jh7110-dphy-tx.c
index c64d1c91b..66473a533 100644
--- a/drivers/phy/starfive/phy-jh7110-dphy-tx.c
+++ b/drivers/phy/starfive/phy-jh7110-dphy-tx.c
@@ -209,7 +209,7 @@ static u32 stf_dphy_get_config_index(u32 bitrate)
 	return 0;
 }
 
-static void stf_dphy_hw_reset(struct stf_dphy *dphy, int assert)
+static int stf_dphy_hw_reset(struct stf_dphy *dphy, int assert)
 {
 	int rc;
 	u32 status = 0;
@@ -224,8 +224,12 @@ static void stf_dphy_hw_reset(struct stf_dphy *dphy, int assert)
 					       !(FIELD_GET(STF_DPHY_RGS_CDTX_PLL_UNLOCK, status)),
 					       STF_DPHY_HW_DELAY_US, STF_DPHY_HW_TIMEOUT_US);
 		if (rc)
-			dev_err(dphy->dev, "MIPI dphy-tx # PLL Locked\n");
+			dev_err(dphy->dev, "MIPI dphy-tx PLL failed to lock\n");
+
+		return rc;
 	}
+
+	return 0;
 }
 
 static int stf_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
@@ -313,7 +317,9 @@ static int stf_dphy_init(struct phy *phy)
 	struct stf_dphy *dphy = phy_get_drvdata(phy);
 	int ret;
 
-	stf_dphy_hw_reset(dphy, 1);
+	ret = stf_dphy_hw_reset(dphy, 1);
+	if (ret)
+		return ret;
 
 	writel(FIELD_PREP(STF_DPHY_SCFG_PPI_C_READY_SEL, 0) |
 	       FIELD_PREP(STF_DPHY_SCFG_DSI_TXREADY_ESC_SEL, 0),
@@ -350,7 +356,9 @@ static int stf_dphy_exit(struct phy *phy)
 
 	clk_disable_unprepare(dphy->txesc_clk);
 
-	stf_dphy_hw_reset(dphy, 0);
+	ret = stf_dphy_hw_reset(dphy, 0);
+	if (ret)
+		return ret;
 
 	return 0;
 }
-- 
2.50.1 (Apple Git-155)


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

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] phy: starfive: jh7110-dphy: fail init when the PLL does not lock
  2026-06-24 14:39 [PATCH] phy: starfive: jh7110-dphy: fail init when the PLL does not lock Pengpeng Hou
@ 2026-06-24 14:48 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-06-24 14:48 UTC (permalink / raw)
  To: Pengpeng Hou; +Cc: linux-phy, olteanv, neil.armstrong, vkoul

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Failed PLL lock leaves the hardware out of reset, preventing clean initialization retries.
--

commit 2ff03101ec29d4c2e1b0317fa73877af8894c09e
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

phy: starfive: jh7110-dphy: fail init when the PLL does not lock
    
This commit updates stf_dphy_hw_reset() to return a timeout error instead
of just logging it when the PHY PLL fails to lock. It also propagates this
error up through the initialization and exit paths so the driver can properly
fail when the lock condition is not met.

> diff --git a/drivers/phy/starfive/phy-jh7110-dphy-tx.c b/drivers/phy/starfive/phy-jh7110-dphy-tx.c
> --- a/drivers/phy/starfive/phy-jh7110-dphy-tx.c
> +++ b/drivers/phy/starfive/phy-jh7110-dphy-tx.c

[ ... ]

> @@ -224,8 +224,12 @@ static void stf_dphy_hw_reset(struct stf_dphy *dphy, int assert)
>  					       !(FIELD_GET(STF_DPHY_RGS_CDTX_PLL_UNLOCK, status)),
>  					       STF_DPHY_HW_DELAY_US, STF_DPHY_HW_TIMEOUT_US);
>  		if (rc)
> -			dev_err(dphy->dev, "MIPI dphy-tx # PLL Locked\n");
> +			dev_err(dphy->dev, "MIPI dphy-tx PLL failed to lock\n");
> +
> +		return rc;
>  	}

[Severity: Medium]
When stf_dphy_hw_reset() is called with assert set to 1, it writes 1 to the
STF_DPHY_RESETB bit to bring the PHY out of reset. If the PLL lock times out,
the function now returns an error early without reverting the register to 0.

Since phy_init() fails, phy_exit() won't be called to clean up. Does this
leave the hardware permanently out of reset in a dirty state? If the hardware
requires a 0-to-1 reset transition to attempt locking again, will subsequent
initialization retries automatically fail because the hardware was never put
back into reset?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624143911.58472-1-pengpeng@iscas.ac.cn?part=1

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-06-24 14:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-24 14:39 [PATCH] phy: starfive: jh7110-dphy: fail init when the PLL does not lock Pengpeng Hou
2026-06-24 14:48 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox