[PATCH v3] usb: dwc3: st: balance reset ownership on deassert failure

[PATCH v3] usb: dwc3: st: balance reset ownership on deassert failure

[Pengpeng Hou]

The ST DWC3 glue ignores reset deassert failures in probe and resume.
It may publish the child device or access the glue registers without both
reset operations having succeeded.

Check both results and unwind DRD setup failures as well. A shared
softreset provider failure must go through undo_softreset: the reset core
has already incremented its deassert count before calling the provider.

Track whether this driver owns the deassert pair so suspend and remove do
not release it again after failed resume has unwound it. This is reference
ownership, not a guarantee about the physical reset-line state. Resume
itself needs no early-return guard: the paired suspend callback clears
the ownership state before resume is entered.

The issue was found by our static-analysis tool and manually reviewed.

Fixes: f83fca0707c6 ("usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC")
Assisted-by: gpt 5
Signed-off-by: Pengpeng Hou <hppiscas@163.com>
---
Changes since v2:
- remove the redundant resets_active early return in resume, as Thinh queried
- keep failed shared deasserts routed through undo_softreset
- retain the independently needed suspend/remove ownership guards
- rebase on current mainline

Previous version:
https://lore.kernel.org/all/20260922004312.3589-1-hppiscas@163.com/

 drivers/usb/dwc3/dwc3-st.c |   43 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
index 5d513decaacd22de15825dc061c2747cf09fef07..1a2f38f9407ae0aea25edd616f8d77f953b3b404 100644
--- a/drivers/usb/dwc3/dwc3-st.c
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -84,6 +84,7 @@
  * @dr_mode:		drd static host/device config
  * @rstc_pwrdn:		rest controller for powerdown signal
  * @rstc_rst:		reset controller for softreset signal
+ * @resets_active:	whether this driver holds its deasserted reset pair
  */
 
 struct st_dwc3 {
@@ -94,6 +95,7 @@ struct st_dwc3 {
 	enum usb_dr_mode dr_mode;
 	struct reset_control *rstc_pwrdn;
 	struct reset_control *rstc_rst;
+	bool resets_active;
 };
 
 static inline u32 st_dwc3_readl(void __iomem *base, u32 offset)
@@ -242,7 +244,9 @@ static int st_dwc3_probe(struct platform_device *pdev)
 				     "could not get power controller\n");
 
 	/* Manage PowerDown */
-	reset_control_deassert(dwc3_data->rstc_pwrdn);
+	ret = reset_control_deassert(dwc3_data->rstc_pwrdn);
+	if (ret)
+		return ret;
 
 	dwc3_data->rstc_rst =
 		devm_reset_control_get_shared(dev, "softreset");
@@ -253,7 +257,10 @@ static int st_dwc3_probe(struct platform_device *pdev)
 	}
 
 	/* Manage SoftReset */
-	reset_control_deassert(dwc3_data->rstc_rst);
+	ret = reset_control_deassert(dwc3_data->rstc_rst);
+	if (ret)
+		goto undo_softreset;
+	dwc3_data->resets_active = true;
 
 	/* Allocate and initialize the core */
 	ret = of_platform_populate(node, NULL, NULL, dev);
@@ -305,16 +312,22 @@ static void st_dwc3_remove(struct platform_device *pdev)
 
 	of_platform_depopulate(&pdev->dev);
 
-	reset_control_assert(dwc3_data->rstc_pwrdn);
-	reset_control_assert(dwc3_data->rstc_rst);
+	if (dwc3_data->resets_active) {
+		reset_control_assert(dwc3_data->rstc_pwrdn);
+		reset_control_assert(dwc3_data->rstc_rst);
+		dwc3_data->resets_active = false;
+	}
 }
 
 static int st_dwc3_suspend(struct device *dev)
 {
 	struct st_dwc3 *dwc3_data = dev_get_drvdata(dev);
 
-	reset_control_assert(dwc3_data->rstc_pwrdn);
-	reset_control_assert(dwc3_data->rstc_rst);
+	if (dwc3_data->resets_active) {
+		reset_control_assert(dwc3_data->rstc_pwrdn);
+		reset_control_assert(dwc3_data->rstc_rst);
+		dwc3_data->resets_active = false;
+	}
 
 	pinctrl_pm_select_sleep_state(dev);
 
@@ -328,19 +341,31 @@ static int st_dwc3_resume(struct device *dev)
 
 	pinctrl_pm_select_default_state(dev);
 
-	reset_control_deassert(dwc3_data->rstc_pwrdn);
-	reset_control_deassert(dwc3_data->rstc_rst);
+	ret = reset_control_deassert(dwc3_data->rstc_pwrdn);
+	if (ret)
+		return ret;
+
+	ret = reset_control_deassert(dwc3_data->rstc_rst);
+	if (ret)
+		goto undo_softreset;
 
 	ret = st_dwc3_drd_init(dwc3_data);
 	if (ret) {
 		dev_err(dev, "drd initialisation failed\n");
-		return ret;
+		goto undo_softreset;
 	}
 
 	/* ST glue logic init */
 	st_dwc3_init(dwc3_data);
+	dwc3_data->resets_active = true;
 
 	return 0;
+
+undo_softreset:
+	/* A shared deassert holds its reference even if the provider fails. */
+	reset_control_assert(dwc3_data->rstc_rst);
+	reset_control_assert(dwc3_data->rstc_pwrdn);
+	return ret;
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(st_dwc3_dev_pm_ops, st_dwc3_suspend, st_dwc3_resume);

base-commit: 4056b3c9d3c92ea45955641dbafeb9b960e146ef

Re: [PATCH v3] usb: dwc3: st: balance reset ownership on deassert failure

[sashiko-bot]

> The ST DWC3 glue ignores reset deassert failures in probe and resume.
> It may publish the child device or access the glue registers without both
> reset operations having succeeded.
> 
> Check both results and unwind DRD setup failures as well. A shared
> softreset provider failure must go through undo_softreset: the reset core
> has already incremented its deassert count before calling the provider.
> 
> Track whether this driver owns the deassert pair so suspend and remove do
> not release it again after failed resume has unwound it. This is reference
> ownership, not a guarantee about the physical reset-line state. Resume
> itself needs no early-return guard: the paired suspend callback clears
> the ownership state before resume is entered.
> 
> The issue was found by our static-analysis tool and manually reviewed.
> 
> Fixes: f83fca0707c6 ("usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC")
> Assisted-by: gpt 5
> Signed-off-by: Pengpeng Hou <hppiscas@163.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014031920.8390-1-hppiscas@163.com?part=1