[PATCH 00/11] USB: dwc3: error handling fixes and cleanups

[PATCH 00/11] USB: dwc3: error handling fixes and cleanups

[Johan Hovold]

When reviewing the dwc3 runtime PM implementation I noticed that the
probe error handling and unbind code was broken. The first two patches
addresses the corresponding imbalances.

The probe error handling has suffered from some bit rot over years and
an attempt to clean it up lead to the realisation that the code dealing
with the "hibernation" feature was both broken and had never been used.
Rather than try to fix up something which has never been used since it
was first merged ten years ago, let's get rid of this dead code until
there is a mainline user (and a complete implementation).

The rest of the series clean up probe and core initialisation by using
descriptive error labels and adding a few helper functions to improve
readability which will hopefully help prevent similar bugs from being
introduced in the future.

Johan


Johan Hovold (11):
  USB: dwc3: fix runtime pm imbalance on probe errors
  USB: dwc3: fix runtime pm imbalance on unbind
  USB: dwc3: disable autosuspend on unbind
  USB: dwc3: gadget: drop dead hibernation code
  USB: dwc3: drop dead hibernation code
  USB: dwc3: clean up probe error labels
  USB: dwc3: clean up phy init error handling
  USB: dwc3: clean up core init error handling
  USB: dwc3: refactor phy handling
  USB: dwc3: refactor clock lookups
  USB: dwc3: clean up probe declarations

 drivers/usb/dwc3/core.c   | 426 ++++++++++++++++----------------------
 drivers/usb/dwc3/core.h   |   8 -
 drivers/usb/dwc3/gadget.c |  46 +---
 3 files changed, 182 insertions(+), 298 deletions(-)

-- 
2.39.2

[PATCH 01/11] USB: dwc3: fix runtime pm imbalance on probe errors

[Johan Hovold]

Make sure not to suspend the device when probe fails to avoid disabling
clocks and phys multiple times.

Fixes: 328082376aea ("usb: dwc3: fix runtime PM in error path")
Cc: stable@vger.kernel.org      # 4.8
Cc: Roger Quadros <rogerq@ti.com>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/usb/dwc3/core.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 476b63618511..5058bd8d56ca 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1883,13 +1883,11 @@ static int dwc3_probe(struct platform_device *pdev)
 	spin_lock_init(&dwc->lock);
 	mutex_init(&dwc->mutex);
 
+	pm_runtime_get_noresume(dev);
 	pm_runtime_set_active(dev);
 	pm_runtime_use_autosuspend(dev);
 	pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY);
 	pm_runtime_enable(dev);
-	ret = pm_runtime_get_sync(dev);
-	if (ret < 0)
-		goto err1;
 
 	pm_runtime_forbid(dev);
 
@@ -1954,12 +1952,10 @@ static int dwc3_probe(struct platform_device *pdev)
 	dwc3_free_event_buffers(dwc);
 
 err2:
-	pm_runtime_allow(&pdev->dev);
-
-err1:
-	pm_runtime_put_sync(&pdev->dev);
-	pm_runtime_disable(&pdev->dev);
-
+	pm_runtime_allow(dev);
+	pm_runtime_disable(dev);
+	pm_runtime_set_suspended(dev);
+	pm_runtime_put_noidle(dev);
 disable_clks:
 	dwc3_clk_disable(dwc);
 assert_reset:
-- 
2.39.2

Re: [PATCH 01/11] USB: dwc3: fix runtime pm imbalance on probe errors

[Thinh Nguyen]

On Tue, Apr 04, 2023, Johan Hovold wrote:
> Make sure not to suspend the device when probe fails to avoid disabling
> clocks and phys multiple times.
> 
> Fixes: 328082376aea ("usb: dwc3: fix runtime PM in error path")
> Cc: stable@vger.kernel.org      # 4.8
> Cc: Roger Quadros <rogerq@ti.com>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/usb/dwc3/core.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 476b63618511..5058bd8d56ca 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1883,13 +1883,11 @@ static int dwc3_probe(struct platform_device *pdev)
>  	spin_lock_init(&dwc->lock);
>  	mutex_init(&dwc->mutex);
>  
> +	pm_runtime_get_noresume(dev);
>  	pm_runtime_set_active(dev);
>  	pm_runtime_use_autosuspend(dev);
>  	pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY);
>  	pm_runtime_enable(dev);
> -	ret = pm_runtime_get_sync(dev);
> -	if (ret < 0)
> -		goto err1;
>  
>  	pm_runtime_forbid(dev);
>  
> @@ -1954,12 +1952,10 @@ static int dwc3_probe(struct platform_device *pdev)
>  	dwc3_free_event_buffers(dwc);
>  
>  err2:
> -	pm_runtime_allow(&pdev->dev);
> -
> -err1:
> -	pm_runtime_put_sync(&pdev->dev);
> -	pm_runtime_disable(&pdev->dev);
> -
> +	pm_runtime_allow(dev);
> +	pm_runtime_disable(dev);
> +	pm_runtime_set_suspended(dev);
> +	pm_runtime_put_noidle(dev);
>  disable_clks:
>  	dwc3_clk_disable(dwc);
>  assert_reset:
> -- 
> 2.39.2
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

[PATCH 02/11] USB: dwc3: fix runtime pm imbalance on unbind

[Johan Hovold]

Make sure to balance the runtime PM usage count on driver unbind by
adding back the pm_runtime_allow() call that had been erroneously
removed.

Fixes: 266d0493900a ("usb: dwc3: core: don't trigger runtime pm when remove driver")
Cc: stable@vger.kernel.org	# 5.9
Cc: Li Jun <jun.li@nxp.com>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/usb/dwc3/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 5058bd8d56ca..9f8c988c25cb 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1979,6 +1979,7 @@ static int dwc3_remove(struct platform_device *pdev)
 	dwc3_core_exit(dwc);
 	dwc3_ulpi_exit(dwc);
 
+	pm_runtime_allow(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
-- 
2.39.2

Re: [PATCH 02/11] USB: dwc3: fix runtime pm imbalance on unbind

[Thinh Nguyen]

On Tue, Apr 04, 2023, Johan Hovold wrote:
> Make sure to balance the runtime PM usage count on driver unbind by
> adding back the pm_runtime_allow() call that had been erroneously
> removed.
> 
> Fixes: 266d0493900a ("usb: dwc3: core: don't trigger runtime pm when remove driver")
> Cc: stable@vger.kernel.org	# 5.9
> Cc: Li Jun <jun.li@nxp.com>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/usb/dwc3/core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 5058bd8d56ca..9f8c988c25cb 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1979,6 +1979,7 @@ static int dwc3_remove(struct platform_device *pdev)
>  	dwc3_core_exit(dwc);
>  	dwc3_ulpi_exit(dwc);
>  
> +	pm_runtime_allow(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  	pm_runtime_put_noidle(&pdev->dev);
>  	pm_runtime_set_suspended(&pdev->dev);
> -- 
> 2.39.2
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

[PATCH 03/11] USB: dwc3: disable autosuspend on unbind

[Johan Hovold]

Add the missing calls to disable autosuspend on probe errors and on
driver unbind.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/usb/dwc3/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 9f8c988c25cb..5b362ed43e7e 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1954,6 +1954,7 @@ static int dwc3_probe(struct platform_device *pdev)
 err2:
 	pm_runtime_allow(dev);
 	pm_runtime_disable(dev);
+	pm_runtime_dont_use_autosuspend(dev);
 	pm_runtime_set_suspended(dev);
 	pm_runtime_put_noidle(dev);
 disable_clks:
@@ -1981,6 +1982,7 @@ static int dwc3_remove(struct platform_device *pdev)
 
 	pm_runtime_allow(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
 
-- 
2.39.2

Re: [PATCH 03/11] USB: dwc3: disable autosuspend on unbind

[Thinh Nguyen]

On Tue, Apr 04, 2023, Johan Hovold wrote:
> Add the missing calls to disable autosuspend on probe errors and on
> driver unbind.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/usb/dwc3/core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 9f8c988c25cb..5b362ed43e7e 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1954,6 +1954,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  err2:
>  	pm_runtime_allow(dev);
>  	pm_runtime_disable(dev);
> +	pm_runtime_dont_use_autosuspend(dev);
>  	pm_runtime_set_suspended(dev);
>  	pm_runtime_put_noidle(dev);
>  disable_clks:
> @@ -1981,6 +1982,7 @@ static int dwc3_remove(struct platform_device *pdev)
>  
>  	pm_runtime_allow(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
> +	pm_runtime_dont_use_autosuspend(&pdev->dev);
>  	pm_runtime_put_noidle(&pdev->dev);
>  	pm_runtime_set_suspended(&pdev->dev);
>  
> -- 
> 2.39.2
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

[PATCH 04/11] USB: dwc3: gadget: drop dead hibernation code

[Johan Hovold]

The hibernation code is broken and has never been enabled in mainline
and should thus be dropped.

Remove the hibernation bits from the gadget code, which effectively
reverts commits e1dadd3b0f27 ("usb: dwc3: workaround: bogus hibernation
events") and 7b2a0368bbc9 ("usb: dwc3: gadget: set KEEP_CONNECT in case
of hibernation") except for the spurious interrupt warning.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/usb/dwc3/gadget.c | 46 +++++----------------------------------
 1 file changed, 6 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index cf5b4f49c3ed..ef51399fd89e 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2478,7 +2478,7 @@ static void __dwc3_gadget_set_speed(struct dwc3 *dwc)
 	dwc3_writel(dwc->regs, DWC3_DCFG, reg);
 }
 
-static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
+static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
 {
 	u32			reg;
 	u32			timeout = 2000;
@@ -2497,17 +2497,11 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
 			reg &= ~DWC3_DCTL_KEEP_CONNECT;
 		reg |= DWC3_DCTL_RUN_STOP;
 
-		if (dwc->has_hibernation)
-			reg |= DWC3_DCTL_KEEP_CONNECT;
-
 		__dwc3_gadget_set_speed(dwc);
 		dwc->pullups_connected = true;
 	} else {
 		reg &= ~DWC3_DCTL_RUN_STOP;
 
-		if (dwc->has_hibernation && !suspend)
-			reg &= ~DWC3_DCTL_KEEP_CONNECT;
-
 		dwc->pullups_connected = false;
 	}
 
@@ -2574,7 +2568,7 @@ static int dwc3_gadget_soft_disconnect(struct dwc3 *dwc)
 	 * remaining event generated by the controller while polling for
 	 * DSTS.DEVCTLHLT.
 	 */
-	return dwc3_gadget_run_stop(dwc, false, false);
+	return dwc3_gadget_run_stop(dwc, false);
 }
 
 static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on)
@@ -2628,7 +2622,7 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on)
 
 		dwc3_event_buffers_setup(dwc);
 		__dwc3_gadget_start(dwc);
-		ret = dwc3_gadget_run_stop(dwc, true, false);
+		ret = dwc3_gadget_run_stop(dwc, true);
 	}
 
 	pm_runtime_put(dwc->dev);
@@ -4195,30 +4189,6 @@ static void dwc3_gadget_suspend_interrupt(struct dwc3 *dwc,
 	dwc->link_state = next;
 }
 
-static void dwc3_gadget_hibernation_interrupt(struct dwc3 *dwc,
-		unsigned int evtinfo)
-{
-	unsigned int is_ss = evtinfo & BIT(4);
-
-	/*
-	 * WORKAROUND: DWC3 revision 2.20a with hibernation support
-	 * have a known issue which can cause USB CV TD.9.23 to fail
-	 * randomly.
-	 *
-	 * Because of this issue, core could generate bogus hibernation
-	 * events which SW needs to ignore.
-	 *
-	 * Refers to:
-	 *
-	 * STAR#9000546576: Device Mode Hibernation: Issue in USB 2.0
-	 * Device Fallback from SuperSpeed
-	 */
-	if (is_ss ^ (dwc->speed == USB_SPEED_SUPER))
-		return;
-
-	/* enter hibernation here */
-}
-
 static void dwc3_gadget_interrupt(struct dwc3 *dwc,
 		const struct dwc3_event_devt *event)
 {
@@ -4236,11 +4206,7 @@ static void dwc3_gadget_interrupt(struct dwc3 *dwc,
 		dwc3_gadget_wakeup_interrupt(dwc);
 		break;
 	case DWC3_DEVICE_EVENT_HIBER_REQ:
-		if (dev_WARN_ONCE(dwc->dev, !dwc->has_hibernation,
-					"unexpected hibernation event\n"))
-			break;
-
-		dwc3_gadget_hibernation_interrupt(dwc, event->event_info);
+		dev_WARN_ONCE(dwc->dev, true, "unexpected hibernation event\n");
 		break;
 	case DWC3_DEVICE_EVENT_LINK_STATUS_CHANGE:
 		dwc3_gadget_linksts_change_interrupt(dwc, event->event_info);
@@ -4584,7 +4550,7 @@ int dwc3_gadget_suspend(struct dwc3 *dwc)
 	if (!dwc->gadget_driver)
 		return 0;
 
-	dwc3_gadget_run_stop(dwc, false, false);
+	dwc3_gadget_run_stop(dwc, false);
 
 	spin_lock_irqsave(&dwc->lock, flags);
 	dwc3_disconnect_gadget(dwc);
@@ -4605,7 +4571,7 @@ int dwc3_gadget_resume(struct dwc3 *dwc)
 	if (ret < 0)
 		goto err0;
 
-	ret = dwc3_gadget_run_stop(dwc, true, false);
+	ret = dwc3_gadget_run_stop(dwc, true);
 	if (ret < 0)
 		goto err1;
 
-- 
2.39.2

Re: [PATCH 04/11] USB: dwc3: gadget: drop dead hibernation code

[Thinh Nguyen]

On Tue, Apr 04, 2023, Johan Hovold wrote:
> The hibernation code is broken and has never been enabled in mainline
> and should thus be dropped.
> 
> Remove the hibernation bits from the gadget code, which effectively
> reverts commits e1dadd3b0f27 ("usb: dwc3: workaround: bogus hibernation
> events") and 7b2a0368bbc9 ("usb: dwc3: gadget: set KEEP_CONNECT in case
> of hibernation") except for the spurious interrupt warning.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/usb/dwc3/gadget.c | 46 +++++----------------------------------
>  1 file changed, 6 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index cf5b4f49c3ed..ef51399fd89e 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2478,7 +2478,7 @@ static void __dwc3_gadget_set_speed(struct dwc3 *dwc)
>  	dwc3_writel(dwc->regs, DWC3_DCFG, reg);
>  }
>  
> -static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
> +static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
>  {
>  	u32			reg;
>  	u32			timeout = 2000;
> @@ -2497,17 +2497,11 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
>  			reg &= ~DWC3_DCTL_KEEP_CONNECT;
>  		reg |= DWC3_DCTL_RUN_STOP;
>  
> -		if (dwc->has_hibernation)
> -			reg |= DWC3_DCTL_KEEP_CONNECT;
> -
>  		__dwc3_gadget_set_speed(dwc);
>  		dwc->pullups_connected = true;
>  	} else {
>  		reg &= ~DWC3_DCTL_RUN_STOP;
>  
> -		if (dwc->has_hibernation && !suspend)
> -			reg &= ~DWC3_DCTL_KEEP_CONNECT;
> -
>  		dwc->pullups_connected = false;
>  	}
>  
> @@ -2574,7 +2568,7 @@ static int dwc3_gadget_soft_disconnect(struct dwc3 *dwc)
>  	 * remaining event generated by the controller while polling for
>  	 * DSTS.DEVCTLHLT.
>  	 */
> -	return dwc3_gadget_run_stop(dwc, false, false);
> +	return dwc3_gadget_run_stop(dwc, false);
>  }
>  
>  static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on)
> @@ -2628,7 +2622,7 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on)
>  
>  		dwc3_event_buffers_setup(dwc);
>  		__dwc3_gadget_start(dwc);
> -		ret = dwc3_gadget_run_stop(dwc, true, false);
> +		ret = dwc3_gadget_run_stop(dwc, true);
>  	}
>  
>  	pm_runtime_put(dwc->dev);
> @@ -4195,30 +4189,6 @@ static void dwc3_gadget_suspend_interrupt(struct dwc3 *dwc,
>  	dwc->link_state = next;
>  }
>  
> -static void dwc3_gadget_hibernation_interrupt(struct dwc3 *dwc,
> -		unsigned int evtinfo)
> -{
> -	unsigned int is_ss = evtinfo & BIT(4);
> -
> -	/*
> -	 * WORKAROUND: DWC3 revision 2.20a with hibernation support
> -	 * have a known issue which can cause USB CV TD.9.23 to fail
> -	 * randomly.
> -	 *
> -	 * Because of this issue, core could generate bogus hibernation
> -	 * events which SW needs to ignore.
> -	 *
> -	 * Refers to:
> -	 *
> -	 * STAR#9000546576: Device Mode Hibernation: Issue in USB 2.0
> -	 * Device Fallback from SuperSpeed
> -	 */
> -	if (is_ss ^ (dwc->speed == USB_SPEED_SUPER))
> -		return;
> -
> -	/* enter hibernation here */
> -}
> -
>  static void dwc3_gadget_interrupt(struct dwc3 *dwc,
>  		const struct dwc3_event_devt *event)
>  {
> @@ -4236,11 +4206,7 @@ static void dwc3_gadget_interrupt(struct dwc3 *dwc,
>  		dwc3_gadget_wakeup_interrupt(dwc);
>  		break;
>  	case DWC3_DEVICE_EVENT_HIBER_REQ:
> -		if (dev_WARN_ONCE(dwc->dev, !dwc->has_hibernation,
> -					"unexpected hibernation event\n"))
> -			break;
> -
> -		dwc3_gadget_hibernation_interrupt(dwc, event->event_info);
> +		dev_WARN_ONCE(dwc->dev, true, "unexpected hibernation event\n");
>  		break;
>  	case DWC3_DEVICE_EVENT_LINK_STATUS_CHANGE:
>  		dwc3_gadget_linksts_change_interrupt(dwc, event->event_info);
> @@ -4584,7 +4550,7 @@ int dwc3_gadget_suspend(struct dwc3 *dwc)
>  	if (!dwc->gadget_driver)
>  		return 0;
>  
> -	dwc3_gadget_run_stop(dwc, false, false);
> +	dwc3_gadget_run_stop(dwc, false);
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	dwc3_disconnect_gadget(dwc);
> @@ -4605,7 +4571,7 @@ int dwc3_gadget_resume(struct dwc3 *dwc)
>  	if (ret < 0)
>  		goto err0;
>  
> -	ret = dwc3_gadget_run_stop(dwc, true, false);
> +	ret = dwc3_gadget_run_stop(dwc, true);
>  	if (ret < 0)
>  		goto err1;
>  
> -- 
> 2.39.2
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thinh

[PATCH 05/11] USB: dwc3: drop dead hibernation code

[Johan Hovold]

The hibernation code is broken and has never been enabled in mainline
and should thus be dropped.

Specifically, the scratch buffer DMA mapping would have been leaked on
every suspend cycle since commit 51f5d49ad6f0 ("usb: dwc3: core:
simplify suspend/resume operations") if this feature was ever enabled.

The related error handling was also broken and could have resulted in
attempts to unmap never mapped buffers, etc.

This effectively revert commit 0ffcaf3798bf ("usb: dwc3: core: allocate
scratch buffers").

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/usb/dwc3/core.c | 103 +---------------------------------------
 drivers/usb/dwc3/core.h |   8 ----
 2 files changed, 1 insertion(+), 110 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 5b362ed43e7e..d837ab511686 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -534,90 +534,6 @@ void dwc3_event_buffers_cleanup(struct dwc3 *dwc)
 	dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), 0);
 }
 
-static int dwc3_alloc_scratch_buffers(struct dwc3 *dwc)
-{
-	if (!dwc->has_hibernation)
-		return 0;
-
-	if (!dwc->nr_scratch)
-		return 0;
-
-	dwc->scratchbuf = kmalloc_array(dwc->nr_scratch,
-			DWC3_SCRATCHBUF_SIZE, GFP_KERNEL);
-	if (!dwc->scratchbuf)
-		return -ENOMEM;
-
-	return 0;
-}
-
-static int dwc3_setup_scratch_buffers(struct dwc3 *dwc)
-{
-	dma_addr_t scratch_addr;
-	u32 param;
-	int ret;
-
-	if (!dwc->has_hibernation)
-		return 0;
-
-	if (!dwc->nr_scratch)
-		return 0;
-
-	 /* should never fall here */
-	if (!WARN_ON(dwc->scratchbuf))
-		return 0;
-
-	scratch_addr = dma_map_single(dwc->sysdev, dwc->scratchbuf,
-			dwc->nr_scratch * DWC3_SCRATCHBUF_SIZE,
-			DMA_BIDIRECTIONAL);
-	if (dma_mapping_error(dwc->sysdev, scratch_addr)) {
-		dev_err(dwc->sysdev, "failed to map scratch buffer\n");
-		ret = -EFAULT;
-		goto err0;
-	}
-
-	dwc->scratch_addr = scratch_addr;
-
-	param = lower_32_bits(scratch_addr);
-
-	ret = dwc3_send_gadget_generic_command(dwc,
-			DWC3_DGCMD_SET_SCRATCHPAD_ADDR_LO, param);
-	if (ret < 0)
-		goto err1;
-
-	param = upper_32_bits(scratch_addr);
-
-	ret = dwc3_send_gadget_generic_command(dwc,
-			DWC3_DGCMD_SET_SCRATCHPAD_ADDR_HI, param);
-	if (ret < 0)
-		goto err1;
-
-	return 0;
-
-err1:
-	dma_unmap_single(dwc->sysdev, dwc->scratch_addr, dwc->nr_scratch *
-			DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL);
-
-err0:
-	return ret;
-}
-
-static void dwc3_free_scratch_buffers(struct dwc3 *dwc)
-{
-	if (!dwc->has_hibernation)
-		return;
-
-	if (!dwc->nr_scratch)
-		return;
-
-	 /* should never fall here */
-	if (!WARN_ON(dwc->scratchbuf))
-		return;
-
-	dma_unmap_single(dwc->sysdev, dwc->scratch_addr, dwc->nr_scratch *
-			DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL);
-	kfree(dwc->scratchbuf);
-}
-
 static void dwc3_core_num_eps(struct dwc3 *dwc)
 {
 	struct dwc3_hwparams	*parms = &dwc->hwparams;
@@ -877,7 +793,6 @@ static bool dwc3_core_is_valid(struct dwc3 *dwc)
 
 static void dwc3_core_setup_global_control(struct dwc3 *dwc)
 {
-	u32 hwparams4 = dwc->hwparams.hwparams4;
 	u32 reg;
 
 	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
@@ -905,9 +820,6 @@ static void dwc3_core_setup_global_control(struct dwc3 *dwc)
 			reg &= ~DWC3_GCTL_DSBLCLKGTNG;
 		break;
 	case DWC3_GHWPARAMS1_EN_PWROPT_HIB:
-		/* enable hibernation here */
-		dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4);
-
 		/*
 		 * REVISIT Enabling this bit so that host-mode hibernation
 		 * will work. Device-mode hibernation is not yet implemented.
@@ -1151,10 +1063,6 @@ static int dwc3_core_init(struct dwc3 *dwc)
 	dwc3_core_setup_global_control(dwc);
 	dwc3_core_num_eps(dwc);
 
-	ret = dwc3_setup_scratch_buffers(dwc);
-	if (ret)
-		goto err1;
-
 	/* Set power down scale of suspend_clk */
 	dwc3_set_power_down_clk_scale(dwc);
 
@@ -1908,14 +1816,10 @@ static int dwc3_probe(struct platform_device *pdev)
 	if (ret)
 		goto err3;
 
-	ret = dwc3_alloc_scratch_buffers(dwc);
-	if (ret)
-		goto err3;
-
 	ret = dwc3_core_init(dwc);
 	if (ret) {
 		dev_err_probe(dev, ret, "failed to initialize core\n");
-		goto err4;
+		goto err3;
 	}
 
 	dwc3_check_params(dwc);
@@ -1944,10 +1848,6 @@ static int dwc3_probe(struct platform_device *pdev)
 	phy_exit(dwc->usb3_generic_phy);
 
 	dwc3_ulpi_exit(dwc);
-
-err4:
-	dwc3_free_scratch_buffers(dwc);
-
 err3:
 	dwc3_free_event_buffers(dwc);
 
@@ -1987,7 +1887,6 @@ static int dwc3_remove(struct platform_device *pdev)
 	pm_runtime_set_suspended(&pdev->dev);
 
 	dwc3_free_event_buffers(dwc);
-	dwc3_free_scratch_buffers(dwc);
 
 	if (dwc->usb_psy)
 		power_supply_put(dwc->usb_psy);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 4743e918dcaf..fbbc565d3b34 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -969,12 +969,10 @@ struct dwc3_scratchpad_array {
  * @drd_work: workqueue used for role swapping
  * @ep0_trb: trb which is used for the ctrl_req
  * @bounce: address of bounce buffer
- * @scratchbuf: address of scratch buffer
  * @setup_buf: used while precessing STD USB requests
  * @ep0_trb_addr: dma address of @ep0_trb
  * @bounce_addr: dma address of @bounce
  * @ep0_usb_req: dummy req used while handling STD USB requests
- * @scratch_addr: dma address of scratchbuf
  * @ep0_in_setup: one control transfer is completed and enter setup phase
  * @lock: for synchronizing
  * @mutex: for mode switching
@@ -999,7 +997,6 @@ struct dwc3_scratchpad_array {
  * @current_otg_role: current role of operation while using the OTG block
  * @desired_otg_role: desired role of operation while using the OTG block
  * @otg_restart_host: flag that OTG controller needs to restart host
- * @nr_scratch: number of scratch buffers
  * @u1u2: only used on revisions <1.83a for workaround
  * @maximum_speed: maximum speed requested (mainly for testing purposes)
  * @max_ssp_rate: SuperSpeed Plus maximum signaling rate and lane count
@@ -1056,7 +1053,6 @@ struct dwc3_scratchpad_array {
  * @delayed_status: true when gadget driver asks for delayed status
  * @ep0_bounced: true when we used bounce buffer
  * @ep0_expect_in: true when we expect a DATA IN transfer
- * @has_hibernation: true when dwc3 was configured with Hibernation
  * @sysdev_is_parent: true when dwc3 device has a parent driver
  * @has_lpm_erratum: true when core was configured with LPM Erratum. Note that
  *			there's now way for software to detect this in runtime.
@@ -1123,11 +1119,9 @@ struct dwc3 {
 	struct work_struct	drd_work;
 	struct dwc3_trb		*ep0_trb;
 	void			*bounce;
-	void			*scratchbuf;
 	u8			*setup_buf;
 	dma_addr_t		ep0_trb_addr;
 	dma_addr_t		bounce_addr;
-	dma_addr_t		scratch_addr;
 	struct dwc3_request	ep0_usb_req;
 	struct completion	ep0_in_setup;
 
@@ -1187,7 +1181,6 @@ struct dwc3 {
 	u32			current_otg_role;
 	u32			desired_otg_role;
 	bool			otg_restart_host;
-	u32			nr_scratch;
 	u32			u1u2;
 	u32			maximum_speed;
 	u32			gadget_max_speed;
@@ -1284,7 +1277,6 @@ struct dwc3 {
 	unsigned		delayed_status:1;
 	unsigned		ep0_bounced:1;
 	unsigned		ep0_expect_in:1;
-	unsigned		has_hibernation:1;
 	unsigned		sysdev_is_parent:1;
 	unsigned		has_lpm_erratum:1;
 	unsigned		is_utmi_l1_suspend:1;
-- 
2.39.2

Re: [PATCH 05/11] USB: dwc3: drop dead hibernation code

[Thinh Nguyen]

On Tue, Apr 04, 2023, Johan Hovold wrote:
> The hibernation code is broken and has never been enabled in mainline
> and should thus be dropped.

Hibernation is not implemented in the kernel mainline. We only have some
small stub/placeholder code for it. It's fine to remove these.

> 
> Specifically, the scratch buffer DMA mapping would have been leaked on
> every suspend cycle since commit 51f5d49ad6f0 ("usb: dwc3: core:
> simplify suspend/resume operations") if this feature was ever enabled.
> 
> The related error handling was also broken and could have resulted in
> attempts to unmap never mapped buffers, etc.
> 
> This effectively revert commit 0ffcaf3798bf ("usb: dwc3: core: allocate
> scratch buffers").
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/usb/dwc3/core.c | 103 +---------------------------------------
>  drivers/usb/dwc3/core.h |   8 ----
>  2 files changed, 1 insertion(+), 110 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 5b362ed43e7e..d837ab511686 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -534,90 +534,6 @@ void dwc3_event_buffers_cleanup(struct dwc3 *dwc)
>  	dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), 0);
>  }
>  
> -static int dwc3_alloc_scratch_buffers(struct dwc3 *dwc)
> -{
> -	if (!dwc->has_hibernation)
> -		return 0;
> -
> -	if (!dwc->nr_scratch)
> -		return 0;
> -
> -	dwc->scratchbuf = kmalloc_array(dwc->nr_scratch,
> -			DWC3_SCRATCHBUF_SIZE, GFP_KERNEL);
> -	if (!dwc->scratchbuf)
> -		return -ENOMEM;
> -
> -	return 0;
> -}
> -
> -static int dwc3_setup_scratch_buffers(struct dwc3 *dwc)
> -{
> -	dma_addr_t scratch_addr;
> -	u32 param;
> -	int ret;
> -
> -	if (!dwc->has_hibernation)
> -		return 0;
> -
> -	if (!dwc->nr_scratch)
> -		return 0;
> -
> -	 /* should never fall here */
> -	if (!WARN_ON(dwc->scratchbuf))
> -		return 0;
> -
> -	scratch_addr = dma_map_single(dwc->sysdev, dwc->scratchbuf,
> -			dwc->nr_scratch * DWC3_SCRATCHBUF_SIZE,
> -			DMA_BIDIRECTIONAL);
> -	if (dma_mapping_error(dwc->sysdev, scratch_addr)) {
> -		dev_err(dwc->sysdev, "failed to map scratch buffer\n");
> -		ret = -EFAULT;
> -		goto err0;
> -	}
> -
> -	dwc->scratch_addr = scratch_addr;
> -
> -	param = lower_32_bits(scratch_addr);
> -
> -	ret = dwc3_send_gadget_generic_command(dwc,
> -			DWC3_DGCMD_SET_SCRATCHPAD_ADDR_LO, param);
> -	if (ret < 0)
> -		goto err1;
> -
> -	param = upper_32_bits(scratch_addr);
> -
> -	ret = dwc3_send_gadget_generic_command(dwc,
> -			DWC3_DGCMD_SET_SCRATCHPAD_ADDR_HI, param);
> -	if (ret < 0)
> -		goto err1;
> -
> -	return 0;
> -
> -err1:
> -	dma_unmap_single(dwc->sysdev, dwc->scratch_addr, dwc->nr_scratch *
> -			DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL);
> -
> -err0:
> -	return ret;
> -}
> -
> -static void dwc3_free_scratch_buffers(struct dwc3 *dwc)
> -{
> -	if (!dwc->has_hibernation)
> -		return;
> -
> -	if (!dwc->nr_scratch)
> -		return;
> -
> -	 /* should never fall here */
> -	if (!WARN_ON(dwc->scratchbuf))
> -		return;
> -
> -	dma_unmap_single(dwc->sysdev, dwc->scratch_addr, dwc->nr_scratch *
> -			DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL);
> -	kfree(dwc->scratchbuf);
> -}
> -
>  static void dwc3_core_num_eps(struct dwc3 *dwc)
>  {
>  	struct dwc3_hwparams	*parms = &dwc->hwparams;
> @@ -877,7 +793,6 @@ static bool dwc3_core_is_valid(struct dwc3 *dwc)
>  
>  static void dwc3_core_setup_global_control(struct dwc3 *dwc)
>  {
> -	u32 hwparams4 = dwc->hwparams.hwparams4;
>  	u32 reg;
>  
>  	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> @@ -905,9 +820,6 @@ static void dwc3_core_setup_global_control(struct dwc3 *dwc)
>  			reg &= ~DWC3_GCTL_DSBLCLKGTNG;
>  		break;
>  	case DWC3_GHWPARAMS1_EN_PWROPT_HIB:
> -		/* enable hibernation here */
> -		dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4);
> -
>  		/*
>  		 * REVISIT Enabling this bit so that host-mode hibernation
>  		 * will work. Device-mode hibernation is not yet implemented.
> @@ -1151,10 +1063,6 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  	dwc3_core_setup_global_control(dwc);
>  	dwc3_core_num_eps(dwc);
>  
> -	ret = dwc3_setup_scratch_buffers(dwc);
> -	if (ret)
> -		goto err1;
> -
>  	/* Set power down scale of suspend_clk */
>  	dwc3_set_power_down_clk_scale(dwc);
>  
> @@ -1908,14 +1816,10 @@ static int dwc3_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err3;
>  
> -	ret = dwc3_alloc_scratch_buffers(dwc);
> -	if (ret)
> -		goto err3;
> -
>  	ret = dwc3_core_init(dwc);
>  	if (ret) {
>  		dev_err_probe(dev, ret, "failed to initialize core\n");
> -		goto err4;
> +		goto err3;
>  	}
>  
>  	dwc3_check_params(dwc);
> @@ -1944,10 +1848,6 @@ static int dwc3_probe(struct platform_device *pdev)
>  	phy_exit(dwc->usb3_generic_phy);
>  
>  	dwc3_ulpi_exit(dwc);
> -
> -err4:
> -	dwc3_free_scratch_buffers(dwc);
> -
>  err3:
>  	dwc3_free_event_buffers(dwc);
>  
> @@ -1987,7 +1887,6 @@ static int dwc3_remove(struct platform_device *pdev)
>  	pm_runtime_set_suspended(&pdev->dev);
>  
>  	dwc3_free_event_buffers(dwc);
> -	dwc3_free_scratch_buffers(dwc);
>  
>  	if (dwc->usb_psy)
>  		power_supply_put(dwc->usb_psy);
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 4743e918dcaf..fbbc565d3b34 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -969,12 +969,10 @@ struct dwc3_scratchpad_array {
>   * @drd_work: workqueue used for role swapping
>   * @ep0_trb: trb which is used for the ctrl_req
>   * @bounce: address of bounce buffer
> - * @scratchbuf: address of scratch buffer
>   * @setup_buf: used while precessing STD USB requests
>   * @ep0_trb_addr: dma address of @ep0_trb
>   * @bounce_addr: dma address of @bounce
>   * @ep0_usb_req: dummy req used while handling STD USB requests
> - * @scratch_addr: dma address of scratchbuf
>   * @ep0_in_setup: one control transfer is completed and enter setup phase
>   * @lock: for synchronizing
>   * @mutex: for mode switching
> @@ -999,7 +997,6 @@ struct dwc3_scratchpad_array {
>   * @current_otg_role: current role of operation while using the OTG block
>   * @desired_otg_role: desired role of operation while using the OTG block
>   * @otg_restart_host: flag that OTG controller needs to restart host
> - * @nr_scratch: number of scratch buffers
>   * @u1u2: only used on revisions <1.83a for workaround
>   * @maximum_speed: maximum speed requested (mainly for testing purposes)
>   * @max_ssp_rate: SuperSpeed Plus maximum signaling rate and lane count
> @@ -1056,7 +1053,6 @@ struct dwc3_scratchpad_array {
>   * @delayed_status: true when gadget driver asks for delayed status
>   * @ep0_bounced: true when we used bounce buffer
>   * @ep0_expect_in: true when we expect a DATA IN transfer
> - * @has_hibernation: true when dwc3 was configured with Hibernation
>   * @sysdev_is_parent: true when dwc3 device has a parent driver
>   * @has_lpm_erratum: true when core was configured with LPM Erratum. Note that
>   *			there's now way for software to detect this in runtime.
> @@ -1123,11 +1119,9 @@ struct dwc3 {
>  	struct work_struct	drd_work;
>  	struct dwc3_trb		*ep0_trb;
>  	void			*bounce;
> -	void			*scratchbuf;
>  	u8			*setup_buf;
>  	dma_addr_t		ep0_trb_addr;
>  	dma_addr_t		bounce_addr;
> -	dma_addr_t		scratch_addr;
>  	struct dwc3_request	ep0_usb_req;
>  	struct completion	ep0_in_setup;
>  
> @@ -1187,7 +1181,6 @@ struct dwc3 {
>  	u32			current_otg_role;
>  	u32			desired_otg_role;
>  	bool			otg_restart_host;
> -	u32			nr_scratch;
>  	u32			u1u2;
>  	u32			maximum_speed;
>  	u32			gadget_max_speed;
> @@ -1284,7 +1277,6 @@ struct dwc3 {
>  	unsigned		delayed_status:1;
>  	unsigned		ep0_bounced:1;
>  	unsigned		ep0_expect_in:1;
> -	unsigned		has_hibernation:1;
>  	unsigned		sysdev_is_parent:1;
>  	unsigned		has_lpm_erratum:1;
>  	unsigned		is_utmi_l1_suspend:1;
> -- 
> 2.39.2
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

[PATCH 06/11] USB: dwc3: clean up probe error labels

[Johan Hovold]

Use descriptive names consistently for the probe error labels.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/usb/dwc3/core.c | 45 ++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index d837ab511686..de84e057d28b 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1705,7 +1705,7 @@ static int dwc3_probe(struct platform_device *pdev)
 	dwc->reset = devm_reset_control_array_get_optional_shared(dev);
 	if (IS_ERR(dwc->reset)) {
 		ret = PTR_ERR(dwc->reset);
-		goto put_usb_psy;
+		goto err_put_psy;
 	}
 
 	if (dev->of_node) {
@@ -1719,7 +1719,7 @@ static int dwc3_probe(struct platform_device *pdev)
 		if (IS_ERR(dwc->bus_clk)) {
 			ret = dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
 					    "could not get bus clock\n");
-			goto put_usb_psy;
+			goto err_put_psy;
 		}
 
 		if (dwc->bus_clk == NULL) {
@@ -1727,7 +1727,7 @@ static int dwc3_probe(struct platform_device *pdev)
 			if (IS_ERR(dwc->bus_clk)) {
 				ret = dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
 						    "could not get bus clock\n");
-				goto put_usb_psy;
+				goto err_put_psy;
 			}
 		}
 
@@ -1735,7 +1735,7 @@ static int dwc3_probe(struct platform_device *pdev)
 		if (IS_ERR(dwc->ref_clk)) {
 			ret = dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
 					    "could not get ref clock\n");
-			goto put_usb_psy;
+			goto err_put_psy;
 		}
 
 		if (dwc->ref_clk == NULL) {
@@ -1743,7 +1743,7 @@ static int dwc3_probe(struct platform_device *pdev)
 			if (IS_ERR(dwc->ref_clk)) {
 				ret = dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
 						    "could not get ref clock\n");
-				goto put_usb_psy;
+				goto err_put_psy;
 			}
 		}
 
@@ -1751,7 +1751,7 @@ static int dwc3_probe(struct platform_device *pdev)
 		if (IS_ERR(dwc->susp_clk)) {
 			ret = dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
 					    "could not get suspend clock\n");
-			goto put_usb_psy;
+			goto err_put_psy;
 		}
 
 		if (dwc->susp_clk == NULL) {
@@ -1759,23 +1759,23 @@ static int dwc3_probe(struct platform_device *pdev)
 			if (IS_ERR(dwc->susp_clk)) {
 				ret = dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
 						    "could not get suspend clock\n");
-				goto put_usb_psy;
+				goto err_put_psy;
 			}
 		}
 	}
 
 	ret = reset_control_deassert(dwc->reset);
 	if (ret)
-		goto put_usb_psy;
+		goto err_put_psy;
 
 	ret = dwc3_clk_enable(dwc);
 	if (ret)
-		goto assert_reset;
+		goto err_assert_reset;
 
 	if (!dwc3_core_is_valid(dwc)) {
 		dev_err(dwc->dev, "this is not a DesignWare USB3 DRD Core\n");
 		ret = -ENODEV;
-		goto disable_clks;
+		goto err_disable_clks;
 	}
 
 	platform_set_drvdata(pdev, dwc);
@@ -1785,7 +1785,7 @@ static int dwc3_probe(struct platform_device *pdev)
 	    DWC3_GHWPARAMS0_AWIDTH(dwc->hwparams.hwparams0) == 64) {
 		ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
 		if (ret)
-			goto disable_clks;
+			goto err_disable_clks;
 	}
 
 	spin_lock_init(&dwc->lock);
@@ -1803,23 +1803,23 @@ static int dwc3_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(dwc->dev, "failed to allocate event buffers\n");
 		ret = -ENOMEM;
-		goto err2;
+		goto err_allow_rpm;
 	}
 
 	dwc->edev = dwc3_get_extcon(dwc);
 	if (IS_ERR(dwc->edev)) {
 		ret = dev_err_probe(dwc->dev, PTR_ERR(dwc->edev), "failed to get extcon\n");
-		goto err3;
+		goto err_free_event_buffers;
 	}
 
 	ret = dwc3_get_dr_mode(dwc);
 	if (ret)
-		goto err3;
+		goto err_free_event_buffers;
 
 	ret = dwc3_core_init(dwc);
 	if (ret) {
 		dev_err_probe(dev, ret, "failed to initialize core\n");
-		goto err3;
+		goto err_free_event_buffers;
 	}
 
 	dwc3_check_params(dwc);
@@ -1827,13 +1827,13 @@ static int dwc3_probe(struct platform_device *pdev)
 
 	ret = dwc3_core_init_mode(dwc);
 	if (ret)
-		goto err5;
+		goto err_exit_debugfs;
 
 	pm_runtime_put(dev);
 
 	return 0;
 
-err5:
+err_exit_debugfs:
 	dwc3_debugfs_exit(dwc);
 	dwc3_event_buffers_cleanup(dwc);
 
@@ -1848,20 +1848,19 @@ static int dwc3_probe(struct platform_device *pdev)
 	phy_exit(dwc->usb3_generic_phy);
 
 	dwc3_ulpi_exit(dwc);
-err3:
+err_free_event_buffers:
 	dwc3_free_event_buffers(dwc);
-
-err2:
+err_allow_rpm:
 	pm_runtime_allow(dev);
 	pm_runtime_disable(dev);
 	pm_runtime_dont_use_autosuspend(dev);
 	pm_runtime_set_suspended(dev);
 	pm_runtime_put_noidle(dev);
-disable_clks:
+err_disable_clks:
 	dwc3_clk_disable(dwc);
-assert_reset:
+err_assert_reset:
 	reset_control_assert(dwc->reset);
-put_usb_psy:
+err_put_psy:
 	if (dwc->usb_psy)
 		power_supply_put(dwc->usb_psy);
 
-- 
2.39.2

Re: [PATCH 06/11] USB: dwc3: clean up probe error labels

[Thinh Nguyen]

On Tue, Apr 04, 2023, Johan Hovold wrote:
> Use descriptive names consistently for the probe error labels.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/usb/dwc3/core.c | 45 ++++++++++++++++++++---------------------
>  1 file changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index d837ab511686..de84e057d28b 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1705,7 +1705,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  	dwc->reset = devm_reset_control_array_get_optional_shared(dev);
>  	if (IS_ERR(dwc->reset)) {
>  		ret = PTR_ERR(dwc->reset);
> -		goto put_usb_psy;
> +		goto err_put_psy;
>  	}
>  
>  	if (dev->of_node) {
> @@ -1719,7 +1719,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  		if (IS_ERR(dwc->bus_clk)) {
>  			ret = dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
>  					    "could not get bus clock\n");
> -			goto put_usb_psy;
> +			goto err_put_psy;
>  		}
>  
>  		if (dwc->bus_clk == NULL) {
> @@ -1727,7 +1727,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  			if (IS_ERR(dwc->bus_clk)) {
>  				ret = dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
>  						    "could not get bus clock\n");
> -				goto put_usb_psy;
> +				goto err_put_psy;
>  			}
>  		}
>  
> @@ -1735,7 +1735,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  		if (IS_ERR(dwc->ref_clk)) {
>  			ret = dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
>  					    "could not get ref clock\n");
> -			goto put_usb_psy;
> +			goto err_put_psy;
>  		}
>  
>  		if (dwc->ref_clk == NULL) {
> @@ -1743,7 +1743,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  			if (IS_ERR(dwc->ref_clk)) {
>  				ret = dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
>  						    "could not get ref clock\n");
> -				goto put_usb_psy;
> +				goto err_put_psy;
>  			}
>  		}
>  
> @@ -1751,7 +1751,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  		if (IS_ERR(dwc->susp_clk)) {
>  			ret = dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
>  					    "could not get suspend clock\n");
> -			goto put_usb_psy;
> +			goto err_put_psy;
>  		}
>  
>  		if (dwc->susp_clk == NULL) {
> @@ -1759,23 +1759,23 @@ static int dwc3_probe(struct platform_device *pdev)
>  			if (IS_ERR(dwc->susp_clk)) {
>  				ret = dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
>  						    "could not get suspend clock\n");
> -				goto put_usb_psy;
> +				goto err_put_psy;
>  			}
>  		}
>  	}
>  
>  	ret = reset_control_deassert(dwc->reset);
>  	if (ret)
> -		goto put_usb_psy;
> +		goto err_put_psy;
>  
>  	ret = dwc3_clk_enable(dwc);
>  	if (ret)
> -		goto assert_reset;
> +		goto err_assert_reset;
>  
>  	if (!dwc3_core_is_valid(dwc)) {
>  		dev_err(dwc->dev, "this is not a DesignWare USB3 DRD Core\n");
>  		ret = -ENODEV;
> -		goto disable_clks;
> +		goto err_disable_clks;
>  	}
>  
>  	platform_set_drvdata(pdev, dwc);
> @@ -1785,7 +1785,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  	    DWC3_GHWPARAMS0_AWIDTH(dwc->hwparams.hwparams0) == 64) {
>  		ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
>  		if (ret)
> -			goto disable_clks;
> +			goto err_disable_clks;
>  	}
>  
>  	spin_lock_init(&dwc->lock);
> @@ -1803,23 +1803,23 @@ static int dwc3_probe(struct platform_device *pdev)
>  	if (ret) {
>  		dev_err(dwc->dev, "failed to allocate event buffers\n");
>  		ret = -ENOMEM;
> -		goto err2;
> +		goto err_allow_rpm;
>  	}
>  
>  	dwc->edev = dwc3_get_extcon(dwc);
>  	if (IS_ERR(dwc->edev)) {
>  		ret = dev_err_probe(dwc->dev, PTR_ERR(dwc->edev), "failed to get extcon\n");
> -		goto err3;
> +		goto err_free_event_buffers;
>  	}
>  
>  	ret = dwc3_get_dr_mode(dwc);
>  	if (ret)
> -		goto err3;
> +		goto err_free_event_buffers;
>  
>  	ret = dwc3_core_init(dwc);
>  	if (ret) {
>  		dev_err_probe(dev, ret, "failed to initialize core\n");
> -		goto err3;
> +		goto err_free_event_buffers;
>  	}
>  
>  	dwc3_check_params(dwc);
> @@ -1827,13 +1827,13 @@ static int dwc3_probe(struct platform_device *pdev)
>  
>  	ret = dwc3_core_init_mode(dwc);
>  	if (ret)
> -		goto err5;
> +		goto err_exit_debugfs;
>  
>  	pm_runtime_put(dev);
>  
>  	return 0;
>  
> -err5:
> +err_exit_debugfs:
>  	dwc3_debugfs_exit(dwc);
>  	dwc3_event_buffers_cleanup(dwc);
>  
> @@ -1848,20 +1848,19 @@ static int dwc3_probe(struct platform_device *pdev)
>  	phy_exit(dwc->usb3_generic_phy);
>  
>  	dwc3_ulpi_exit(dwc);
> -err3:
> +err_free_event_buffers:
>  	dwc3_free_event_buffers(dwc);
> -
> -err2:
> +err_allow_rpm:
>  	pm_runtime_allow(dev);
>  	pm_runtime_disable(dev);
>  	pm_runtime_dont_use_autosuspend(dev);
>  	pm_runtime_set_suspended(dev);
>  	pm_runtime_put_noidle(dev);
> -disable_clks:
> +err_disable_clks:
>  	dwc3_clk_disable(dwc);
> -assert_reset:
> +err_assert_reset:
>  	reset_control_assert(dwc->reset);
> -put_usb_psy:
> +err_put_psy:
>  	if (dwc->usb_psy)
>  		power_supply_put(dwc->usb_psy);
>  
> -- 
> 2.39.2
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

[PATCH 07/11] USB: dwc3: clean up phy init error handling

[Johan Hovold]

While there likely are no platforms out there that mix generic and
legacy PHYs the driver should still be able to handle that, if only for
consistency reasons.

Add the missing calls to shutdown any legacy PHYs if generic PHY
initialisation fails.

Note that we continue to happily ignore potential errors from the legacy
PHY callbacks...

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/usb/dwc3/core.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index de84e057d28b..15405f1f7aef 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1031,15 +1031,14 @@ static int dwc3_core_init(struct dwc3 *dwc)
 
 	usb_phy_init(dwc->usb2_phy);
 	usb_phy_init(dwc->usb3_phy);
+
 	ret = phy_init(dwc->usb2_generic_phy);
 	if (ret < 0)
-		goto err0a;
+		goto err_shutdown_usb3_phy;
 
 	ret = phy_init(dwc->usb3_generic_phy);
-	if (ret < 0) {
-		phy_exit(dwc->usb2_generic_phy);
-		goto err0a;
-	}
+	if (ret < 0)
+		goto err_exit_usb2_phy;
 
 	ret = dwc3_core_soft_reset(dwc);
 	if (ret)
@@ -1215,11 +1214,12 @@ static int dwc3_core_init(struct dwc3 *dwc)
 	usb_phy_set_suspend(dwc->usb3_phy, 1);
 
 err1:
-	usb_phy_shutdown(dwc->usb2_phy);
-	usb_phy_shutdown(dwc->usb3_phy);
-	phy_exit(dwc->usb2_generic_phy);
 	phy_exit(dwc->usb3_generic_phy);
-
+err_exit_usb2_phy:
+	phy_exit(dwc->usb2_generic_phy);
+err_shutdown_usb3_phy:
+	usb_phy_shutdown(dwc->usb3_phy);
+	usb_phy_shutdown(dwc->usb2_phy);
 err0a:
 	dwc3_ulpi_exit(dwc);
 
-- 
2.39.2

Re: [PATCH 07/11] USB: dwc3: clean up phy init error handling

[Thinh Nguyen]

On Tue, Apr 04, 2023, Johan Hovold wrote:
> While there likely are no platforms out there that mix generic and
> legacy PHYs the driver should still be able to handle that, if only for
> consistency reasons.
> 
> Add the missing calls to shutdown any legacy PHYs if generic PHY
> initialisation fails.
> 
> Note that we continue to happily ignore potential errors from the legacy
> PHY callbacks...
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/usb/dwc3/core.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index de84e057d28b..15405f1f7aef 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1031,15 +1031,14 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  
>  	usb_phy_init(dwc->usb2_phy);
>  	usb_phy_init(dwc->usb3_phy);
> +
>  	ret = phy_init(dwc->usb2_generic_phy);
>  	if (ret < 0)
> -		goto err0a;
> +		goto err_shutdown_usb3_phy;
>  
>  	ret = phy_init(dwc->usb3_generic_phy);
> -	if (ret < 0) {
> -		phy_exit(dwc->usb2_generic_phy);
> -		goto err0a;
> -	}
> +	if (ret < 0)
> +		goto err_exit_usb2_phy;
>  
>  	ret = dwc3_core_soft_reset(dwc);
>  	if (ret)
> @@ -1215,11 +1214,12 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  	usb_phy_set_suspend(dwc->usb3_phy, 1);
>  
>  err1:
> -	usb_phy_shutdown(dwc->usb2_phy);
> -	usb_phy_shutdown(dwc->usb3_phy);
> -	phy_exit(dwc->usb2_generic_phy);
>  	phy_exit(dwc->usb3_generic_phy);
> -
> +err_exit_usb2_phy:
> +	phy_exit(dwc->usb2_generic_phy);
> +err_shutdown_usb3_phy:
> +	usb_phy_shutdown(dwc->usb3_phy);
> +	usb_phy_shutdown(dwc->usb2_phy);
>  err0a:
>  	dwc3_ulpi_exit(dwc);
>  
> -- 
> 2.39.2
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

[PATCH 08/11] USB: dwc3: clean up core init error handling

[Johan Hovold]

Clean up the core init error handling by using descriptive names for the
error labels and releasing resourcing in reverse order consistently.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/usb/dwc3/core.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 15405f1f7aef..c499ef026500 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1008,7 +1008,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
 
 	ret = dwc3_phy_setup(dwc);
 	if (ret)
-		goto err0;
+		return ret;
 
 	if (!dwc->ulpi_ready) {
 		ret = dwc3_core_ulpi_init(dwc);
@@ -1017,7 +1017,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
 				dwc3_core_soft_reset(dwc);
 				ret = -EPROBE_DEFER;
 			}
-			goto err0;
+			return ret;
 		}
 		dwc->ulpi_ready = true;
 	}
@@ -1025,7 +1025,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
 	if (!dwc->phys_ready) {
 		ret = dwc3_core_get_phy(dwc);
 		if (ret)
-			goto err0a;
+			goto err_exit_ulpi;
 		dwc->phys_ready = true;
 	}
 
@@ -1042,7 +1042,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
 
 	ret = dwc3_core_soft_reset(dwc);
 	if (ret)
-		goto err1;
+		goto err_exit_usb3_phy;
 
 	if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
 	    !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) {
@@ -1077,16 +1077,16 @@ static int dwc3_core_init(struct dwc3 *dwc)
 	usb_phy_set_suspend(dwc->usb3_phy, 0);
 	ret = phy_power_on(dwc->usb2_generic_phy);
 	if (ret < 0)
-		goto err2;
+		goto err_suspend_usb3_phy;
 
 	ret = phy_power_on(dwc->usb3_generic_phy);
 	if (ret < 0)
-		goto err3;
+		goto err_power_off_usb2_phy;
 
 	ret = dwc3_event_buffers_setup(dwc);
 	if (ret) {
 		dev_err(dwc->dev, "failed to setup event buffers\n");
-		goto err4;
+		goto err_power_off_usb3_phy;
 	}
 
 	/*
@@ -1203,27 +1203,23 @@ static int dwc3_core_init(struct dwc3 *dwc)
 
 	return 0;
 
-err4:
+err_power_off_usb3_phy:
 	phy_power_off(dwc->usb3_generic_phy);
-
-err3:
+err_power_off_usb2_phy:
 	phy_power_off(dwc->usb2_generic_phy);
-
-err2:
-	usb_phy_set_suspend(dwc->usb2_phy, 1);
+err_suspend_usb3_phy:
 	usb_phy_set_suspend(dwc->usb3_phy, 1);
-
-err1:
+	usb_phy_set_suspend(dwc->usb2_phy, 1);
+err_exit_usb3_phy:
 	phy_exit(dwc->usb3_generic_phy);
 err_exit_usb2_phy:
 	phy_exit(dwc->usb2_generic_phy);
 err_shutdown_usb3_phy:
 	usb_phy_shutdown(dwc->usb3_phy);
 	usb_phy_shutdown(dwc->usb2_phy);
-err0a:
+err_exit_ulpi:
 	dwc3_ulpi_exit(dwc);
 
-err0:
 	return ret;
 }
 
-- 
2.39.2

Re: [PATCH 08/11] USB: dwc3: clean up core init error handling

[Thinh Nguyen]

On Tue, Apr 04, 2023, Johan Hovold wrote:
> Clean up the core init error handling by using descriptive names for the
> error labels and releasing resourcing in reverse order consistently.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/usb/dwc3/core.c | 30 +++++++++++++-----------------
>  1 file changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 15405f1f7aef..c499ef026500 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1008,7 +1008,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  
>  	ret = dwc3_phy_setup(dwc);
>  	if (ret)
> -		goto err0;
> +		return ret;
>  
>  	if (!dwc->ulpi_ready) {
>  		ret = dwc3_core_ulpi_init(dwc);
> @@ -1017,7 +1017,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  				dwc3_core_soft_reset(dwc);
>  				ret = -EPROBE_DEFER;
>  			}
> -			goto err0;
> +			return ret;
>  		}
>  		dwc->ulpi_ready = true;
>  	}
> @@ -1025,7 +1025,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  	if (!dwc->phys_ready) {
>  		ret = dwc3_core_get_phy(dwc);
>  		if (ret)
> -			goto err0a;
> +			goto err_exit_ulpi;
>  		dwc->phys_ready = true;
>  	}
>  
> @@ -1042,7 +1042,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  
>  	ret = dwc3_core_soft_reset(dwc);
>  	if (ret)
> -		goto err1;
> +		goto err_exit_usb3_phy;
>  
>  	if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
>  	    !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) {
> @@ -1077,16 +1077,16 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  	usb_phy_set_suspend(dwc->usb3_phy, 0);
>  	ret = phy_power_on(dwc->usb2_generic_phy);
>  	if (ret < 0)
> -		goto err2;
> +		goto err_suspend_usb3_phy;
>  
>  	ret = phy_power_on(dwc->usb3_generic_phy);
>  	if (ret < 0)
> -		goto err3;
> +		goto err_power_off_usb2_phy;
>  
>  	ret = dwc3_event_buffers_setup(dwc);
>  	if (ret) {
>  		dev_err(dwc->dev, "failed to setup event buffers\n");
> -		goto err4;
> +		goto err_power_off_usb3_phy;
>  	}
>  
>  	/*
> @@ -1203,27 +1203,23 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  
>  	return 0;
>  
> -err4:
> +err_power_off_usb3_phy:
>  	phy_power_off(dwc->usb3_generic_phy);
> -
> -err3:
> +err_power_off_usb2_phy:
>  	phy_power_off(dwc->usb2_generic_phy);
> -
> -err2:
> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
> +err_suspend_usb3_phy:
>  	usb_phy_set_suspend(dwc->usb3_phy, 1);
> -
> -err1:
> +	usb_phy_set_suspend(dwc->usb2_phy, 1);
> +err_exit_usb3_phy:
>  	phy_exit(dwc->usb3_generic_phy);
>  err_exit_usb2_phy:
>  	phy_exit(dwc->usb2_generic_phy);
>  err_shutdown_usb3_phy:
>  	usb_phy_shutdown(dwc->usb3_phy);
>  	usb_phy_shutdown(dwc->usb2_phy);
> -err0a:
> +err_exit_ulpi:
>  	dwc3_ulpi_exit(dwc);
>  
> -err0:
>  	return ret;
>  }
>  
> -- 
> 2.39.2
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

[PATCH 09/11] USB: dwc3: refactor phy handling

[Johan Hovold]

Refactor the PHY handling using four new helpers to initialise,
deinitialise, power on and power off all the PHYs.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/usb/dwc3/core.c | 143 ++++++++++++++++++++++++----------------
 1 file changed, 86 insertions(+), 57 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index c499ef026500..52cd5ddfebd5 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -721,6 +721,76 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
 	return 0;
 }
 
+static int dwc3_phy_init(struct dwc3 *dwc)
+{
+	int ret;
+
+	usb_phy_init(dwc->usb2_phy);
+	usb_phy_init(dwc->usb3_phy);
+
+	ret = phy_init(dwc->usb2_generic_phy);
+	if (ret < 0)
+		goto err_shutdown_usb3_phy;
+
+	ret = phy_init(dwc->usb3_generic_phy);
+	if (ret < 0)
+		goto err_exit_usb2_phy;
+
+	return 0;
+
+err_exit_usb2_phy:
+	phy_exit(dwc->usb2_generic_phy);
+err_shutdown_usb3_phy:
+	usb_phy_shutdown(dwc->usb3_phy);
+	usb_phy_shutdown(dwc->usb2_phy);
+
+	return ret;
+}
+
+static void dwc3_phy_exit(struct dwc3 *dwc)
+{
+	phy_exit(dwc->usb3_generic_phy);
+	phy_exit(dwc->usb2_generic_phy);
+
+	usb_phy_shutdown(dwc->usb3_phy);
+	usb_phy_shutdown(dwc->usb2_phy);
+}
+
+static int dwc3_phy_power_on(struct dwc3 *dwc)
+{
+	int ret;
+
+	usb_phy_set_suspend(dwc->usb2_phy, 0);
+	usb_phy_set_suspend(dwc->usb3_phy, 0);
+
+	ret = phy_power_on(dwc->usb2_generic_phy);
+	if (ret < 0)
+		goto err_suspend_usb3_phy;
+
+	ret = phy_power_on(dwc->usb3_generic_phy);
+	if (ret < 0)
+		goto err_power_off_usb2_phy;
+
+	return 0;
+
+err_power_off_usb2_phy:
+	phy_power_off(dwc->usb2_generic_phy);
+err_suspend_usb3_phy:
+	usb_phy_set_suspend(dwc->usb3_phy, 1);
+	usb_phy_set_suspend(dwc->usb2_phy, 1);
+
+	return ret;
+}
+
+static void dwc3_phy_power_off(struct dwc3 *dwc)
+{
+	phy_power_off(dwc->usb3_generic_phy);
+	phy_power_off(dwc->usb2_generic_phy);
+
+	usb_phy_set_suspend(dwc->usb3_phy, 1);
+	usb_phy_set_suspend(dwc->usb2_phy, 1);
+}
+
 static int dwc3_clk_enable(struct dwc3 *dwc)
 {
 	int ret;
@@ -756,17 +826,8 @@ static void dwc3_clk_disable(struct dwc3 *dwc)
 static void dwc3_core_exit(struct dwc3 *dwc)
 {
 	dwc3_event_buffers_cleanup(dwc);
-
-	usb_phy_set_suspend(dwc->usb2_phy, 1);
-	usb_phy_set_suspend(dwc->usb3_phy, 1);
-	phy_power_off(dwc->usb2_generic_phy);
-	phy_power_off(dwc->usb3_generic_phy);
-
-	usb_phy_shutdown(dwc->usb2_phy);
-	usb_phy_shutdown(dwc->usb3_phy);
-	phy_exit(dwc->usb2_generic_phy);
-	phy_exit(dwc->usb3_generic_phy);
-
+	dwc3_phy_power_off(dwc);
+	dwc3_phy_exit(dwc);
 	dwc3_clk_disable(dwc);
 	reset_control_assert(dwc->reset);
 }
@@ -1029,20 +1090,13 @@ static int dwc3_core_init(struct dwc3 *dwc)
 		dwc->phys_ready = true;
 	}
 
-	usb_phy_init(dwc->usb2_phy);
-	usb_phy_init(dwc->usb3_phy);
-
-	ret = phy_init(dwc->usb2_generic_phy);
-	if (ret < 0)
-		goto err_shutdown_usb3_phy;
-
-	ret = phy_init(dwc->usb3_generic_phy);
-	if (ret < 0)
-		goto err_exit_usb2_phy;
+	ret = dwc3_phy_init(dwc);
+	if (ret)
+		goto err_exit_ulpi;
 
 	ret = dwc3_core_soft_reset(dwc);
 	if (ret)
-		goto err_exit_usb3_phy;
+		goto err_exit_phy;
 
 	if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
 	    !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) {
@@ -1073,20 +1127,14 @@ static int dwc3_core_init(struct dwc3 *dwc)
 
 	dwc3_set_incr_burst_type(dwc);
 
-	usb_phy_set_suspend(dwc->usb2_phy, 0);
-	usb_phy_set_suspend(dwc->usb3_phy, 0);
-	ret = phy_power_on(dwc->usb2_generic_phy);
-	if (ret < 0)
-		goto err_suspend_usb3_phy;
-
-	ret = phy_power_on(dwc->usb3_generic_phy);
-	if (ret < 0)
-		goto err_power_off_usb2_phy;
+	dwc3_phy_power_on(dwc);
+	if (ret)
+		goto err_exit_phy;
 
 	ret = dwc3_event_buffers_setup(dwc);
 	if (ret) {
 		dev_err(dwc->dev, "failed to setup event buffers\n");
-		goto err_power_off_usb3_phy;
+		goto err_power_off_phy;
 	}
 
 	/*
@@ -1203,20 +1251,10 @@ static int dwc3_core_init(struct dwc3 *dwc)
 
 	return 0;
 
-err_power_off_usb3_phy:
-	phy_power_off(dwc->usb3_generic_phy);
-err_power_off_usb2_phy:
-	phy_power_off(dwc->usb2_generic_phy);
-err_suspend_usb3_phy:
-	usb_phy_set_suspend(dwc->usb3_phy, 1);
-	usb_phy_set_suspend(dwc->usb2_phy, 1);
-err_exit_usb3_phy:
-	phy_exit(dwc->usb3_generic_phy);
-err_exit_usb2_phy:
-	phy_exit(dwc->usb2_generic_phy);
-err_shutdown_usb3_phy:
-	usb_phy_shutdown(dwc->usb3_phy);
-	usb_phy_shutdown(dwc->usb2_phy);
+err_power_off_phy:
+	dwc3_phy_power_off(dwc);
+err_exit_phy:
+	dwc3_phy_exit(dwc);
 err_exit_ulpi:
 	dwc3_ulpi_exit(dwc);
 
@@ -1832,17 +1870,8 @@ static int dwc3_probe(struct platform_device *pdev)
 err_exit_debugfs:
 	dwc3_debugfs_exit(dwc);
 	dwc3_event_buffers_cleanup(dwc);
-
-	usb_phy_set_suspend(dwc->usb2_phy, 1);
-	usb_phy_set_suspend(dwc->usb3_phy, 1);
-	phy_power_off(dwc->usb2_generic_phy);
-	phy_power_off(dwc->usb3_generic_phy);
-
-	usb_phy_shutdown(dwc->usb2_phy);
-	usb_phy_shutdown(dwc->usb3_phy);
-	phy_exit(dwc->usb2_generic_phy);
-	phy_exit(dwc->usb3_generic_phy);
-
+	dwc3_phy_power_off(dwc);
+	dwc3_phy_exit(dwc);
 	dwc3_ulpi_exit(dwc);
 err_free_event_buffers:
 	dwc3_free_event_buffers(dwc);
-- 
2.39.2

Re: [PATCH 09/11] USB: dwc3: refactor phy handling

[Thinh Nguyen]

On Tue, Apr 04, 2023, Johan Hovold wrote:
> Refactor the PHY handling using four new helpers to initialise,
> deinitialise, power on and power off all the PHYs.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/usb/dwc3/core.c | 143 ++++++++++++++++++++++++----------------
>  1 file changed, 86 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index c499ef026500..52cd5ddfebd5 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -721,6 +721,76 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>  	return 0;
>  }
>  
> +static int dwc3_phy_init(struct dwc3 *dwc)
> +{
> +	int ret;
> +
> +	usb_phy_init(dwc->usb2_phy);
> +	usb_phy_init(dwc->usb3_phy);
> +
> +	ret = phy_init(dwc->usb2_generic_phy);
> +	if (ret < 0)
> +		goto err_shutdown_usb3_phy;
> +
> +	ret = phy_init(dwc->usb3_generic_phy);
> +	if (ret < 0)
> +		goto err_exit_usb2_phy;
> +
> +	return 0;
> +
> +err_exit_usb2_phy:
> +	phy_exit(dwc->usb2_generic_phy);
> +err_shutdown_usb3_phy:
> +	usb_phy_shutdown(dwc->usb3_phy);
> +	usb_phy_shutdown(dwc->usb2_phy);
> +
> +	return ret;
> +}
> +
> +static void dwc3_phy_exit(struct dwc3 *dwc)
> +{
> +	phy_exit(dwc->usb3_generic_phy);
> +	phy_exit(dwc->usb2_generic_phy);
> +
> +	usb_phy_shutdown(dwc->usb3_phy);
> +	usb_phy_shutdown(dwc->usb2_phy);
> +}
> +
> +static int dwc3_phy_power_on(struct dwc3 *dwc)
> +{
> +	int ret;
> +
> +	usb_phy_set_suspend(dwc->usb2_phy, 0);
> +	usb_phy_set_suspend(dwc->usb3_phy, 0);
> +
> +	ret = phy_power_on(dwc->usb2_generic_phy);
> +	if (ret < 0)
> +		goto err_suspend_usb3_phy;
> +
> +	ret = phy_power_on(dwc->usb3_generic_phy);
> +	if (ret < 0)
> +		goto err_power_off_usb2_phy;
> +
> +	return 0;
> +
> +err_power_off_usb2_phy:
> +	phy_power_off(dwc->usb2_generic_phy);
> +err_suspend_usb3_phy:
> +	usb_phy_set_suspend(dwc->usb3_phy, 1);
> +	usb_phy_set_suspend(dwc->usb2_phy, 1);
> +
> +	return ret;
> +}
> +
> +static void dwc3_phy_power_off(struct dwc3 *dwc)
> +{
> +	phy_power_off(dwc->usb3_generic_phy);
> +	phy_power_off(dwc->usb2_generic_phy);
> +
> +	usb_phy_set_suspend(dwc->usb3_phy, 1);
> +	usb_phy_set_suspend(dwc->usb2_phy, 1);
> +}
> +
>  static int dwc3_clk_enable(struct dwc3 *dwc)
>  {
>  	int ret;
> @@ -756,17 +826,8 @@ static void dwc3_clk_disable(struct dwc3 *dwc)
>  static void dwc3_core_exit(struct dwc3 *dwc)
>  {
>  	dwc3_event_buffers_cleanup(dwc);
> -
> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
> -	usb_phy_set_suspend(dwc->usb3_phy, 1);
> -	phy_power_off(dwc->usb2_generic_phy);
> -	phy_power_off(dwc->usb3_generic_phy);
> -
> -	usb_phy_shutdown(dwc->usb2_phy);
> -	usb_phy_shutdown(dwc->usb3_phy);
> -	phy_exit(dwc->usb2_generic_phy);
> -	phy_exit(dwc->usb3_generic_phy);
> -
> +	dwc3_phy_power_off(dwc);
> +	dwc3_phy_exit(dwc);
>  	dwc3_clk_disable(dwc);
>  	reset_control_assert(dwc->reset);
>  }
> @@ -1029,20 +1090,13 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  		dwc->phys_ready = true;
>  	}
>  
> -	usb_phy_init(dwc->usb2_phy);
> -	usb_phy_init(dwc->usb3_phy);
> -
> -	ret = phy_init(dwc->usb2_generic_phy);
> -	if (ret < 0)
> -		goto err_shutdown_usb3_phy;
> -
> -	ret = phy_init(dwc->usb3_generic_phy);
> -	if (ret < 0)
> -		goto err_exit_usb2_phy;
> +	ret = dwc3_phy_init(dwc);
> +	if (ret)
> +		goto err_exit_ulpi;
>  
>  	ret = dwc3_core_soft_reset(dwc);
>  	if (ret)
> -		goto err_exit_usb3_phy;
> +		goto err_exit_phy;
>  
>  	if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
>  	    !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) {
> @@ -1073,20 +1127,14 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  
>  	dwc3_set_incr_burst_type(dwc);
>  
> -	usb_phy_set_suspend(dwc->usb2_phy, 0);
> -	usb_phy_set_suspend(dwc->usb3_phy, 0);
> -	ret = phy_power_on(dwc->usb2_generic_phy);
> -	if (ret < 0)
> -		goto err_suspend_usb3_phy;
> -
> -	ret = phy_power_on(dwc->usb3_generic_phy);
> -	if (ret < 0)
> -		goto err_power_off_usb2_phy;
> +	dwc3_phy_power_on(dwc);
> +	if (ret)
> +		goto err_exit_phy;
>  
>  	ret = dwc3_event_buffers_setup(dwc);
>  	if (ret) {
>  		dev_err(dwc->dev, "failed to setup event buffers\n");
> -		goto err_power_off_usb3_phy;
> +		goto err_power_off_phy;
>  	}
>  
>  	/*
> @@ -1203,20 +1251,10 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  
>  	return 0;
>  
> -err_power_off_usb3_phy:
> -	phy_power_off(dwc->usb3_generic_phy);
> -err_power_off_usb2_phy:
> -	phy_power_off(dwc->usb2_generic_phy);
> -err_suspend_usb3_phy:
> -	usb_phy_set_suspend(dwc->usb3_phy, 1);
> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
> -err_exit_usb3_phy:
> -	phy_exit(dwc->usb3_generic_phy);
> -err_exit_usb2_phy:
> -	phy_exit(dwc->usb2_generic_phy);
> -err_shutdown_usb3_phy:
> -	usb_phy_shutdown(dwc->usb3_phy);
> -	usb_phy_shutdown(dwc->usb2_phy);
> +err_power_off_phy:
> +	dwc3_phy_power_off(dwc);
> +err_exit_phy:
> +	dwc3_phy_exit(dwc);
>  err_exit_ulpi:
>  	dwc3_ulpi_exit(dwc);
>  
> @@ -1832,17 +1870,8 @@ static int dwc3_probe(struct platform_device *pdev)
>  err_exit_debugfs:
>  	dwc3_debugfs_exit(dwc);
>  	dwc3_event_buffers_cleanup(dwc);
> -
> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
> -	usb_phy_set_suspend(dwc->usb3_phy, 1);
> -	phy_power_off(dwc->usb2_generic_phy);
> -	phy_power_off(dwc->usb3_generic_phy);
> -
> -	usb_phy_shutdown(dwc->usb2_phy);
> -	usb_phy_shutdown(dwc->usb3_phy);
> -	phy_exit(dwc->usb2_generic_phy);
> -	phy_exit(dwc->usb3_generic_phy);
> -
> +	dwc3_phy_power_off(dwc);
> +	dwc3_phy_exit(dwc);
>  	dwc3_ulpi_exit(dwc);
>  err_free_event_buffers:
>  	dwc3_free_event_buffers(dwc);
> -- 
> 2.39.2
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

[PATCH 10/11] USB: dwc3: refactor clock lookups

[Johan Hovold]

The probe callback has become unwieldy so break out the clock lookups
into a new helper function to improve readability.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/usb/dwc3/core.c | 116 +++++++++++++++++++++-------------------
 1 file changed, 61 insertions(+), 55 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 52cd5ddfebd5..08432e109a3f 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1692,6 +1692,64 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
 	return edev;
 }
 
+static int dwc3_get_clocks(struct dwc3 *dwc)
+{
+	struct device *dev = dwc->dev;
+
+	if (!dev->of_node)
+		return 0;
+
+	/*
+	 * Clocks are optional, but new DT platforms should support all clocks
+	 * as required by the DT-binding.
+	 * Some devices have different clock names in legacy device trees,
+	 * check for them to retain backwards compatibility.
+	 */
+	dwc->bus_clk = devm_clk_get_optional(dev, "bus_early");
+	if (IS_ERR(dwc->bus_clk)) {
+		return dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
+				"could not get bus clock\n");
+	}
+
+	if (dwc->bus_clk == NULL) {
+		dwc->bus_clk = devm_clk_get_optional(dev, "bus_clk");
+		if (IS_ERR(dwc->bus_clk)) {
+			return dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
+					"could not get bus clock\n");
+		}
+	}
+
+	dwc->ref_clk = devm_clk_get_optional(dev, "ref");
+	if (IS_ERR(dwc->ref_clk)) {
+		return dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
+				"could not get ref clock\n");
+	}
+
+	if (dwc->ref_clk == NULL) {
+		dwc->ref_clk = devm_clk_get_optional(dev, "ref_clk");
+		if (IS_ERR(dwc->ref_clk)) {
+			return dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
+					"could not get ref clock\n");
+		}
+	}
+
+	dwc->susp_clk = devm_clk_get_optional(dev, "suspend");
+	if (IS_ERR(dwc->susp_clk)) {
+		return dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
+				"could not get suspend clock\n");
+	}
+
+	if (dwc->susp_clk == NULL) {
+		dwc->susp_clk = devm_clk_get_optional(dev, "suspend_clk");
+		if (IS_ERR(dwc->susp_clk)) {
+			return dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
+					"could not get suspend clock\n");
+		}
+	}
+
+	return 0;
+}
+
 static int dwc3_probe(struct platform_device *pdev)
 {
 	struct device		*dev = &pdev->dev;
@@ -1742,61 +1800,9 @@ static int dwc3_probe(struct platform_device *pdev)
 		goto err_put_psy;
 	}
 
-	if (dev->of_node) {
-		/*
-		 * Clocks are optional, but new DT platforms should support all
-		 * clocks as required by the DT-binding.
-		 * Some devices have different clock names in legacy device trees,
-		 * check for them to retain backwards compatibility.
-		 */
-		dwc->bus_clk = devm_clk_get_optional(dev, "bus_early");
-		if (IS_ERR(dwc->bus_clk)) {
-			ret = dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
-					    "could not get bus clock\n");
-			goto err_put_psy;
-		}
-
-		if (dwc->bus_clk == NULL) {
-			dwc->bus_clk = devm_clk_get_optional(dev, "bus_clk");
-			if (IS_ERR(dwc->bus_clk)) {
-				ret = dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
-						    "could not get bus clock\n");
-				goto err_put_psy;
-			}
-		}
-
-		dwc->ref_clk = devm_clk_get_optional(dev, "ref");
-		if (IS_ERR(dwc->ref_clk)) {
-			ret = dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
-					    "could not get ref clock\n");
-			goto err_put_psy;
-		}
-
-		if (dwc->ref_clk == NULL) {
-			dwc->ref_clk = devm_clk_get_optional(dev, "ref_clk");
-			if (IS_ERR(dwc->ref_clk)) {
-				ret = dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
-						    "could not get ref clock\n");
-				goto err_put_psy;
-			}
-		}
-
-		dwc->susp_clk = devm_clk_get_optional(dev, "suspend");
-		if (IS_ERR(dwc->susp_clk)) {
-			ret = dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
-					    "could not get suspend clock\n");
-			goto err_put_psy;
-		}
-
-		if (dwc->susp_clk == NULL) {
-			dwc->susp_clk = devm_clk_get_optional(dev, "suspend_clk");
-			if (IS_ERR(dwc->susp_clk)) {
-				ret = dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
-						    "could not get suspend clock\n");
-				goto err_put_psy;
-			}
-		}
-	}
+	ret = dwc3_get_clocks(dwc);
+	if (ret)
+		goto err_put_psy;
 
 	ret = reset_control_deassert(dwc->reset);
 	if (ret)
-- 
2.39.2

Re: [PATCH 10/11] USB: dwc3: refactor clock lookups

[Thinh Nguyen]

On Tue, Apr 04, 2023, Johan Hovold wrote:
> The probe callback has become unwieldy so break out the clock lookups
> into a new helper function to improve readability.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/usb/dwc3/core.c | 116 +++++++++++++++++++++-------------------
>  1 file changed, 61 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 52cd5ddfebd5..08432e109a3f 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1692,6 +1692,64 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
>  	return edev;
>  }
>  
> +static int dwc3_get_clocks(struct dwc3 *dwc)
> +{
> +	struct device *dev = dwc->dev;
> +
> +	if (!dev->of_node)
> +		return 0;
> +
> +	/*
> +	 * Clocks are optional, but new DT platforms should support all clocks
> +	 * as required by the DT-binding.
> +	 * Some devices have different clock names in legacy device trees,
> +	 * check for them to retain backwards compatibility.
> +	 */
> +	dwc->bus_clk = devm_clk_get_optional(dev, "bus_early");
> +	if (IS_ERR(dwc->bus_clk)) {
> +		return dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
> +				"could not get bus clock\n");
> +	}
> +
> +	if (dwc->bus_clk == NULL) {
> +		dwc->bus_clk = devm_clk_get_optional(dev, "bus_clk");
> +		if (IS_ERR(dwc->bus_clk)) {
> +			return dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
> +					"could not get bus clock\n");
> +		}
> +	}
> +
> +	dwc->ref_clk = devm_clk_get_optional(dev, "ref");
> +	if (IS_ERR(dwc->ref_clk)) {
> +		return dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
> +				"could not get ref clock\n");
> +	}
> +
> +	if (dwc->ref_clk == NULL) {
> +		dwc->ref_clk = devm_clk_get_optional(dev, "ref_clk");
> +		if (IS_ERR(dwc->ref_clk)) {
> +			return dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
> +					"could not get ref clock\n");
> +		}
> +	}
> +
> +	dwc->susp_clk = devm_clk_get_optional(dev, "suspend");
> +	if (IS_ERR(dwc->susp_clk)) {
> +		return dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
> +				"could not get suspend clock\n");
> +	}
> +
> +	if (dwc->susp_clk == NULL) {
> +		dwc->susp_clk = devm_clk_get_optional(dev, "suspend_clk");
> +		if (IS_ERR(dwc->susp_clk)) {
> +			return dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
> +					"could not get suspend clock\n");
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int dwc3_probe(struct platform_device *pdev)
>  {
>  	struct device		*dev = &pdev->dev;
> @@ -1742,61 +1800,9 @@ static int dwc3_probe(struct platform_device *pdev)
>  		goto err_put_psy;
>  	}
>  
> -	if (dev->of_node) {
> -		/*
> -		 * Clocks are optional, but new DT platforms should support all
> -		 * clocks as required by the DT-binding.
> -		 * Some devices have different clock names in legacy device trees,
> -		 * check for them to retain backwards compatibility.
> -		 */
> -		dwc->bus_clk = devm_clk_get_optional(dev, "bus_early");
> -		if (IS_ERR(dwc->bus_clk)) {
> -			ret = dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
> -					    "could not get bus clock\n");
> -			goto err_put_psy;
> -		}
> -
> -		if (dwc->bus_clk == NULL) {
> -			dwc->bus_clk = devm_clk_get_optional(dev, "bus_clk");
> -			if (IS_ERR(dwc->bus_clk)) {
> -				ret = dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
> -						    "could not get bus clock\n");
> -				goto err_put_psy;
> -			}
> -		}
> -
> -		dwc->ref_clk = devm_clk_get_optional(dev, "ref");
> -		if (IS_ERR(dwc->ref_clk)) {
> -			ret = dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
> -					    "could not get ref clock\n");
> -			goto err_put_psy;
> -		}
> -
> -		if (dwc->ref_clk == NULL) {
> -			dwc->ref_clk = devm_clk_get_optional(dev, "ref_clk");
> -			if (IS_ERR(dwc->ref_clk)) {
> -				ret = dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
> -						    "could not get ref clock\n");
> -				goto err_put_psy;
> -			}
> -		}
> -
> -		dwc->susp_clk = devm_clk_get_optional(dev, "suspend");
> -		if (IS_ERR(dwc->susp_clk)) {
> -			ret = dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
> -					    "could not get suspend clock\n");
> -			goto err_put_psy;
> -		}
> -
> -		if (dwc->susp_clk == NULL) {
> -			dwc->susp_clk = devm_clk_get_optional(dev, "suspend_clk");
> -			if (IS_ERR(dwc->susp_clk)) {
> -				ret = dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
> -						    "could not get suspend clock\n");
> -				goto err_put_psy;
> -			}
> -		}
> -	}
> +	ret = dwc3_get_clocks(dwc);
> +	if (ret)
> +		goto err_put_psy;
>  
>  	ret = reset_control_deassert(dwc->reset);
>  	if (ret)
> -- 
> 2.39.2
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

[PATCH 11/11] USB: dwc3: clean up probe declarations

[Johan Hovold]

Clean up the probe variable declarations by removing the stray newlines.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/usb/dwc3/core.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 08432e109a3f..24d395b8868c 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1754,12 +1754,10 @@ static int dwc3_probe(struct platform_device *pdev)
 {
 	struct device		*dev = &pdev->dev;
 	struct resource		*res, dwc_res;
+	void __iomem		*regs;
 	struct dwc3		*dwc;
-
 	int			ret;
 
-	void __iomem		*regs;
-
 	dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
 	if (!dwc)
 		return -ENOMEM;
-- 
2.39.2

Re: [PATCH 11/11] USB: dwc3: clean up probe declarations

[Thinh Nguyen]

On Tue, Apr 04, 2023, Johan Hovold wrote:
> Clean up the probe variable declarations by removing the stray newlines.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/usb/dwc3/core.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 08432e109a3f..24d395b8868c 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1754,12 +1754,10 @@ static int dwc3_probe(struct platform_device *pdev)
>  {
>  	struct device		*dev = &pdev->dev;
>  	struct resource		*res, dwc_res;
> +	void __iomem		*regs;
>  	struct dwc3		*dwc;
> -
>  	int			ret;
>  
> -	void __iomem		*regs;
> -
>  	dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
>  	if (!dwc)
>  		return -ENOMEM;
> -- 
> 2.39.2
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

Re: [PATCH 00/11] USB: dwc3: error handling fixes and cleanups

[Thinh Nguyen]

On Tue, Apr 04, 2023, Johan Hovold wrote:
> When reviewing the dwc3 runtime PM implementation I noticed that the
> probe error handling and unbind code was broken. The first two patches
> addresses the corresponding imbalances.
> 
> The probe error handling has suffered from some bit rot over years and
> an attempt to clean it up lead to the realisation that the code dealing
> with the "hibernation" feature was both broken and had never been used.
> Rather than try to fix up something which has never been used since it
> was first merged ten years ago, let's get rid of this dead code until
> there is a mainline user (and a complete implementation).
> 
> The rest of the series clean up probe and core initialisation by using
> descriptive error labels and adding a few helper functions to improve
> readability which will hopefully help prevent similar bugs from being
> introduced in the future.
> 
> Johan
> 
> 
> Johan Hovold (11):
>   USB: dwc3: fix runtime pm imbalance on probe errors
>   USB: dwc3: fix runtime pm imbalance on unbind
>   USB: dwc3: disable autosuspend on unbind
>   USB: dwc3: gadget: drop dead hibernation code
>   USB: dwc3: drop dead hibernation code
>   USB: dwc3: clean up probe error labels
>   USB: dwc3: clean up phy init error handling
>   USB: dwc3: clean up core init error handling
>   USB: dwc3: refactor phy handling
>   USB: dwc3: refactor clock lookups
>   USB: dwc3: clean up probe declarations
> 
>  drivers/usb/dwc3/core.c   | 426 ++++++++++++++++----------------------
>  drivers/usb/dwc3/core.h   |   8 -
>  drivers/usb/dwc3/gadget.c |  46 +---
>  3 files changed, 182 insertions(+), 298 deletions(-)
> 
> -- 
> 2.39.2
> 

Thanks for the cleanup work. I've reviewed some patches, but still need
to spend some more time reviewing the runtime changes.

Thanks,
Thinh