[RFC PATCH] phy: qcom: ipq4019-usb: Propagate reset operation failures

[RFC PATCH] phy: qcom: ipq4019-usb: Propagate reset operation failures

[Pengpeng Hou]

The IPQ4019 USB PHY power operations ignore reset assertion and
deassertion failures.  Their callers can therefore publish a PHY power
transition after the reset controller rejected part of it.

Propagate each reset error and restore the reset that was changed
earlier in the same HS transition where possible.  This is an RFC
because a failed reset operation can leave hardware state uncertain;
feedback is requested on whether the proposed best-effort rollback
matches these PHYs.

The issue was identified via static analysis and manually reviewed.

Assisted-by: LLM
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c | 46 ++++++++++++++++-----
 1 file changed, 36 insertions(+), 10 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
index da6f290af722..8c1834cd6a8e 100644
--- a/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
+++ b/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
@@ -30,7 +30,12 @@ static int ipq4019_ss_phy_power_off(struct phy *_phy)
 {
 	struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
 
-	reset_control_assert(phy->por_rst);
+	int ret;
+
+	ret = reset_control_assert(phy->por_rst);
+	if (ret)
+		return ret;
+
 	msleep(10);
 
 	return 0;
@@ -40,11 +45,13 @@ static int ipq4019_ss_phy_power_on(struct phy *_phy)
 {
 	struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
 
-	ipq4019_ss_phy_power_off(_phy);
+	int ret;
 
-	reset_control_deassert(phy->por_rst);
+	ret = ipq4019_ss_phy_power_off(_phy);
+	if (ret)
+		return ret;
 
-	return 0;
+	return reset_control_deassert(phy->por_rst);
 }
 
 static const struct phy_ops ipq4019_usb_ss_phy_ops = {
@@ -56,10 +63,20 @@ static int ipq4019_hs_phy_power_off(struct phy *_phy)
 {
 	struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
 
-	reset_control_assert(phy->por_rst);
+	int ret;
+
+	ret = reset_control_assert(phy->por_rst);
+	if (ret)
+		return ret;
+
 	msleep(10);
 
-	reset_control_assert(phy->srif_rst);
+	ret = reset_control_assert(phy->srif_rst);
+	if (ret) {
+		reset_control_deassert(phy->por_rst);
+		return ret;
+	}
+
 	msleep(10);
 
 	return 0;
@@ -69,14 +86,23 @@ static int ipq4019_hs_phy_power_on(struct phy *_phy)
 {
 	struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
 
-	ipq4019_hs_phy_power_off(_phy);
+	int ret;
+
+	ret = ipq4019_hs_phy_power_off(_phy);
+	if (ret)
+		return ret;
+
+	ret = reset_control_deassert(phy->srif_rst);
+	if (ret)
+		return ret;
 
-	reset_control_deassert(phy->srif_rst);
 	msleep(10);
 
-	reset_control_deassert(phy->por_rst);
+	ret = reset_control_deassert(phy->por_rst);
+	if (ret)
+		reset_control_assert(phy->srif_rst);
 
-	return 0;
+	return ret;
 }
 
 static const struct phy_ops ipq4019_usb_hs_phy_ops = {
-- 
2.43.0

Re: [RFC PATCH] phy: qcom: ipq4019-usb: Propagate reset operation failures

[Konrad Dybcio]

On 8/28/26 11:18 AM, Pengpeng Hou wrote:
> The IPQ4019 USB PHY power operations ignore reset assertion and
> deassertion failures.  Their callers can therefore publish a PHY power
> transition after the reset controller rejected part of it.
> 
> Propagate each reset error and restore the reset that was changed
> earlier in the same HS transition where possible.  This is an RFC
> because a failed reset operation can leave hardware state uncertain;
> feedback is requested on whether the proposed best-effort rollback
> matches these PHYs.

[...]

> 
> The issue was identified via static analysis and manually reviewed.
> 
> Assisted-by: LLM
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c | 46 ++++++++++++++++-----
>  1 file changed, 36 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
> index da6f290af722..8c1834cd6a8e 100644
> --- a/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
> +++ b/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
> @@ -30,7 +30,12 @@ static int ipq4019_ss_phy_power_off(struct phy *_phy)
>  {
>  	struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
>  
> -	reset_control_assert(phy->por_rst);
> +	int ret;

Please don't leave the blank lines above 'int ret'

[...]

> @@ -69,14 +86,23 @@ static int ipq4019_hs_phy_power_on(struct phy *_phy)
>  {
>  	struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
>  
> -	ipq4019_hs_phy_power_off(_phy);
> +	int ret;
> +
> +	ret = ipq4019_hs_phy_power_off(_phy);
> +	if (ret)
> +		return ret;
> +
> +	ret = reset_control_deassert(phy->srif_rst);
> +	if (ret)
> +		return ret;
>  
> -	reset_control_deassert(phy->srif_rst);
>  	msleep(10);
>  
> -	reset_control_deassert(phy->por_rst);
> +	ret = reset_control_deassert(phy->por_rst);
> +	if (ret)
> +		reset_control_assert(phy->srif_rst);

This return value is silently ignored

Konrad