diff mbox series

[v3,13/14] usb: dwc2: Fix partial power down exiting by system resume

Message ID 20210408094607.1A9BAA0094@mailhost.synopsys.com
State Superseded
Headers show
Series usb: dwc2: Fix Partial Power down issues. | expand

Commit Message

Artur Petrosyan April 8, 2021, 9:46 a.m. UTC
Fixes the implementation of exiting from partial power down
power saving mode when PC is resumed.

Added port connection status checking which prevents exiting from
Partial Power Down mode from _dwc2_hcd_resume() if not in Partial
Power Down mode.

Rearranged the implementation to get rid of many "if"
statements.

NOTE: Switch case statement is used for hibernation partial
power down and clock gating mode determination. In this patch
only Partial Power Down is implemented the Hibernation and
clock gating implementations are planned to be added.

Cc: <stable@vger.kernel.org>
Fixes: 6f6d70597c15 ("usb: dwc2: bus suspend/resume for hosts with DWC2_POWER_DOWN_PARAM_NONE")
Signed-off-by: Artur Petrosyan <Arthur.Petrosyan@synopsys.com>
---
 Changes in v3:
 - None
 Changes in v2:
 - None

 drivers/usb/dwc2/hcd.c | 90 +++++++++++++++++++++---------------------
 1 file changed, 46 insertions(+), 44 deletions(-)

Comments

Minas Harutyunyan April 8, 2021, 1:40 p.m. UTC | #1
On 4/8/2021 1:46 PM, Artur Petrosyan wrote:
> Fixes the implementation of exiting from partial power down

> power saving mode when PC is resumed.

> 

> Added port connection status checking which prevents exiting from

> Partial Power Down mode from _dwc2_hcd_resume() if not in Partial

> Power Down mode.

> 

> Rearranged the implementation to get rid of many "if"

> statements.

> 

> NOTE: Switch case statement is used for hibernation partial

> power down and clock gating mode determination. In this patch

> only Partial Power Down is implemented the Hibernation and

> clock gating implementations are planned to be added.

> 

> Cc: <stable@vger.kernel.org>

> Fixes: 6f6d70597c15 ("usb: dwc2: bus suspend/resume for hosts with DWC2_POWER_DOWN_PARAM_NONE")

> Signed-off-by: Artur Petrosyan <Arthur.Petrosyan@synopsys.com>


Acked-by: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>


> ---

>   Changes in v3:

>   - None

>   Changes in v2:

>   - None

> 

>   drivers/usb/dwc2/hcd.c | 90 +++++++++++++++++++++---------------------

>   1 file changed, 46 insertions(+), 44 deletions(-)

> 

> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c

> index 34030bafdff4..f096006df96f 100644

> --- a/drivers/usb/dwc2/hcd.c

> +++ b/drivers/usb/dwc2/hcd.c

> @@ -4427,7 +4427,7 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)

>   {

>   	struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);

>   	unsigned long flags;

> -	u32 pcgctl;

> +	u32 hprt0;

>   	int ret = 0;

>   

>   	spin_lock_irqsave(&hsotg->lock, flags);

> @@ -4438,11 +4438,40 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)

>   	if (hsotg->lx_state != DWC2_L2)

>   		goto unlock;

>   

> -	if (hsotg->params.power_down > DWC2_POWER_DOWN_PARAM_PARTIAL) {

> +	hprt0 = dwc2_read_hprt0(hsotg);

> +

> +	/*

> +	 * Added port connection status checking which prevents exiting from

> +	 * Partial Power Down mode from _dwc2_hcd_resume() if not in Partial

> +	 * Power Down mode.

> +	 */

> +	if (hprt0 & HPRT0_CONNSTS) {

> +		hsotg->lx_state = DWC2_L0;

> +		goto unlock;

> +	}

> +

> +	switch (hsotg->params.power_down) {

> +	case DWC2_POWER_DOWN_PARAM_PARTIAL:

> +		ret = dwc2_exit_partial_power_down(hsotg, 0, true);

> +		if (ret)

> +			dev_err(hsotg->dev,

> +				"exit partial_power_down failed\n");

> +		/*

> +		 * Set HW accessible bit before powering on the controller

> +		 * since an interrupt may rise.

> +		 */

> +		set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);

> +		break;

> +	case DWC2_POWER_DOWN_PARAM_HIBERNATION:

> +	case DWC2_POWER_DOWN_PARAM_NONE:

> +	default:

>   		hsotg->lx_state = DWC2_L0;

>   		goto unlock;

>   	}

>   

> +	/* Change Root port status, as port status change occurred after resume.*/

> +	hsotg->flags.b.port_suspend_change = 1;

> +

>   	/*

>   	 * Enable power if not already done.

>   	 * This must not be spinlocked since duration

> @@ -4454,52 +4483,25 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)

>   		spin_lock_irqsave(&hsotg->lock, flags);

>   	}

>   

> -	if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {

> -		/*

> -		 * Set HW accessible bit before powering on the controller

> -		 * since an interrupt may rise.

> -		 */

> -		set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);

> -

> -

> -		/* Exit partial_power_down */

> -		ret = dwc2_exit_partial_power_down(hsotg, 0, true);

> -		if (ret && (ret != -ENOTSUPP))

> -			dev_err(hsotg->dev, "exit partial_power_down failed\n");

> -	} else {

> -		pcgctl = readl(hsotg->regs + PCGCTL);

> -		pcgctl &= ~PCGCTL_STOPPCLK;

> -		writel(pcgctl, hsotg->regs + PCGCTL);

> -	}

> -

> -	hsotg->lx_state = DWC2_L0;

> -

> +	/* Enable external vbus supply after resuming the port. */

>   	spin_unlock_irqrestore(&hsotg->lock, flags);

> +	dwc2_vbus_supply_init(hsotg);

>   

> -	if (hsotg->bus_suspended) {

> -		spin_lock_irqsave(&hsotg->lock, flags);

> -		hsotg->flags.b.port_suspend_change = 1;

> -		spin_unlock_irqrestore(&hsotg->lock, flags);

> -		dwc2_port_resume(hsotg);

> -	} else {

> -		if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {

> -			dwc2_vbus_supply_init(hsotg);

> -

> -			/* Wait for controller to correctly update D+/D- level */

> -			usleep_range(3000, 5000);

> -		}

> +	/* Wait for controller to correctly update D+/D- level */

> +	usleep_range(3000, 5000);

> +	spin_lock_irqsave(&hsotg->lock, flags);

>   

> -		/*

> -		 * Clear Port Enable and Port Status changes.

> -		 * Enable Port Power.

> -		 */

> -		dwc2_writel(hsotg, HPRT0_PWR | HPRT0_CONNDET |

> -				HPRT0_ENACHG, HPRT0);

> -		/* Wait for controller to detect Port Connect */

> -		usleep_range(5000, 7000);

> -	}

> +	/*

> +	 * Clear Port Enable and Port Status changes.

> +	 * Enable Port Power.

> +	 */

> +	dwc2_writel(hsotg, HPRT0_PWR | HPRT0_CONNDET |

> +			HPRT0_ENACHG, HPRT0);

>   

> -	return ret;

> +	/* Wait for controller to detect Port Connect */

> +	spin_unlock_irqrestore(&hsotg->lock, flags);

> +	usleep_range(5000, 7000);

> +	spin_lock_irqsave(&hsotg->lock, flags);

>   unlock:

>   	spin_unlock_irqrestore(&hsotg->lock, flags);

>   

>
diff mbox series

Patch

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 34030bafdff4..f096006df96f 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4427,7 +4427,7 @@  static int _dwc2_hcd_resume(struct usb_hcd *hcd)
 {
 	struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
 	unsigned long flags;
-	u32 pcgctl;
+	u32 hprt0;
 	int ret = 0;
 
 	spin_lock_irqsave(&hsotg->lock, flags);
@@ -4438,11 +4438,40 @@  static int _dwc2_hcd_resume(struct usb_hcd *hcd)
 	if (hsotg->lx_state != DWC2_L2)
 		goto unlock;
 
-	if (hsotg->params.power_down > DWC2_POWER_DOWN_PARAM_PARTIAL) {
+	hprt0 = dwc2_read_hprt0(hsotg);
+
+	/*
+	 * Added port connection status checking which prevents exiting from
+	 * Partial Power Down mode from _dwc2_hcd_resume() if not in Partial
+	 * Power Down mode.
+	 */
+	if (hprt0 & HPRT0_CONNSTS) {
+		hsotg->lx_state = DWC2_L0;
+		goto unlock;
+	}
+
+	switch (hsotg->params.power_down) {
+	case DWC2_POWER_DOWN_PARAM_PARTIAL:
+		ret = dwc2_exit_partial_power_down(hsotg, 0, true);
+		if (ret)
+			dev_err(hsotg->dev,
+				"exit partial_power_down failed\n");
+		/*
+		 * Set HW accessible bit before powering on the controller
+		 * since an interrupt may rise.
+		 */
+		set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
+		break;
+	case DWC2_POWER_DOWN_PARAM_HIBERNATION:
+	case DWC2_POWER_DOWN_PARAM_NONE:
+	default:
 		hsotg->lx_state = DWC2_L0;
 		goto unlock;
 	}
 
+	/* Change Root port status, as port status change occurred after resume.*/
+	hsotg->flags.b.port_suspend_change = 1;
+
 	/*
 	 * Enable power if not already done.
 	 * This must not be spinlocked since duration
@@ -4454,52 +4483,25 @@  static int _dwc2_hcd_resume(struct usb_hcd *hcd)
 		spin_lock_irqsave(&hsotg->lock, flags);
 	}
 
-	if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
-		/*
-		 * Set HW accessible bit before powering on the controller
-		 * since an interrupt may rise.
-		 */
-		set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
-
-
-		/* Exit partial_power_down */
-		ret = dwc2_exit_partial_power_down(hsotg, 0, true);
-		if (ret && (ret != -ENOTSUPP))
-			dev_err(hsotg->dev, "exit partial_power_down failed\n");
-	} else {
-		pcgctl = readl(hsotg->regs + PCGCTL);
-		pcgctl &= ~PCGCTL_STOPPCLK;
-		writel(pcgctl, hsotg->regs + PCGCTL);
-	}
-
-	hsotg->lx_state = DWC2_L0;
-
+	/* Enable external vbus supply after resuming the port. */
 	spin_unlock_irqrestore(&hsotg->lock, flags);
+	dwc2_vbus_supply_init(hsotg);
 
-	if (hsotg->bus_suspended) {
-		spin_lock_irqsave(&hsotg->lock, flags);
-		hsotg->flags.b.port_suspend_change = 1;
-		spin_unlock_irqrestore(&hsotg->lock, flags);
-		dwc2_port_resume(hsotg);
-	} else {
-		if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
-			dwc2_vbus_supply_init(hsotg);
-
-			/* Wait for controller to correctly update D+/D- level */
-			usleep_range(3000, 5000);
-		}
+	/* Wait for controller to correctly update D+/D- level */
+	usleep_range(3000, 5000);
+	spin_lock_irqsave(&hsotg->lock, flags);
 
-		/*
-		 * Clear Port Enable and Port Status changes.
-		 * Enable Port Power.
-		 */
-		dwc2_writel(hsotg, HPRT0_PWR | HPRT0_CONNDET |
-				HPRT0_ENACHG, HPRT0);
-		/* Wait for controller to detect Port Connect */
-		usleep_range(5000, 7000);
-	}
+	/*
+	 * Clear Port Enable and Port Status changes.
+	 * Enable Port Power.
+	 */
+	dwc2_writel(hsotg, HPRT0_PWR | HPRT0_CONNDET |
+			HPRT0_ENACHG, HPRT0);
 
-	return ret;
+	/* Wait for controller to detect Port Connect */
+	spin_unlock_irqrestore(&hsotg->lock, flags);
+	usleep_range(5000, 7000);
+	spin_lock_irqsave(&hsotg->lock, flags);
 unlock:
 	spin_unlock_irqrestore(&hsotg->lock, flags);