[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

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

[Pengpeng Hou]

Hi Konrad,

Your comments on the declaration layout and unchecked rollback return
are valid. Rechecking the provider exposed a more fundamental limitation
in the RFC's premise: the in-tree IPQ4019 DT uses GCC resets, and
qcom_reset_set_assert() returns zero after the regmap operations.

I therefore do not have evidence that the hardware-failure case described
in the RFC is reported through this provider. Generic reset API errors
alone do not establish that case on this platform.

Please keep this RFC on hold. I will not respin it as a demonstrated
IPQ4019 bug fix based only on the local return-value checks.

Thanks,
Pengpeng

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

[Konrad Dybcio]

On 10/14/26 4:51 AM, Pengpeng Hou wrote:
> Hi Konrad,
> 
> Your comments on the declaration layout and unchecked rollback return
> are valid. Rechecking the provider exposed a more fundamental limitation
> in the RFC's premise: the in-tree IPQ4019 DT uses GCC resets, and
> qcom_reset_set_assert() returns zero after the regmap operations.
> 
> I therefore do not have evidence that the hardware-failure case described
> in the RFC is reported through this provider. Generic reset API errors
> alone do not establish that case on this platform.
> 
> Please keep this RFC on hold. I will not respin it as a demonstrated
> IPQ4019 bug fix based only on the local return-value checks.

Please tune your agent better.

Konrad