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

[Philipp Zabel]

On Mi, 2026-10-14 at 11:19 +0800, Pengpeng Hou wrote:
> 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@xxxxxxx>
> ---
> 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@xxxxxxx/
> 
>  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;

		goto undo_powerdown;

This behaviour of _deassert causing workarounds in drivers is
unfortunate.

Maybe we should add a reset_control_deassert_get/put() pair for shared
resets where _get() decrements deassert_count on failure and _put()
returns void.

regards
Philipp