diff mbox series

[v2] usb: dwc3-am62: Fix up wake-up configuration and spurious wake up

Message ID 20230324114429.21838-1-rogerq@kernel.org
State New
Headers
Series [v2] usb: dwc3-am62: Fix up wake-up configuration and spurious wake up |

Commit Message

Roger Quadros March 24, 2023, 11:44 a.m. UTC 
  Explicitly set and clear wakeup config so we don't leave anything
to chance.

Clear wakeup status on suspend so we know what caused wake up.

The LINESTATE wake up should not be enabled in device mode
if we are not connected to a USB host and in USB suspend (U2/L3)
else it will cause spurious wake up.

For now, don't enable LINESTATE. This means wake up from
USB resume will not work but at least we won't have any spurious
wake ups.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
Changelog:
v2: don't enable LINESTATE wake-up at all in device mode.

 drivers/usb/dwc3/dwc3-am62.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

Comments

Thinh Nguyen March 24, 2023, 6:36 p.m. UTC | #1 
On Fri, Mar 24, 2023, Roger Quadros wrote:
> Explicitly set and clear wakeup config so we don't leave anything
> to chance.
> 
> Clear wakeup status on suspend so we know what caused wake up.
> 
> The LINESTATE wake up should not be enabled in device mode
> if we are not connected to a USB host and in USB suspend (U2/L3)
> else it will cause spurious wake up.
> 
> For now, don't enable LINESTATE. This means wake up from
> USB resume will not work but at least we won't have any spurious
> wake ups.
> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
> Changelog:
> v2: don't enable LINESTATE wake-up at all in device mode.
> 
>  drivers/usb/dwc3/dwc3-am62.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c
> index 859b48279658..b22fb78bc8e7 100644
> --- a/drivers/usb/dwc3/dwc3-am62.c
> +++ b/drivers/usb/dwc3/dwc3-am62.c
> @@ -60,6 +60,13 @@
>  #define USBSS_WAKEUP_CFG_SESSVALID_EN	BIT(1)
>  #define USBSS_WAKEUP_CFG_VBUSVALID_EN	BIT(0)
>  
> +#define USBSS_WAKEUP_CFG_ALL	(USBSS_WAKEUP_CFG_VBUSVALID_EN | \
> +				 USBSS_WAKEUP_CFG_SESSVALID_EN | \
> +				 USBSS_WAKEUP_CFG_LINESTATE_EN | \
> +				 USBSS_WAKEUP_CFG_OVERCURRENT_EN)
> +
> +#define USBSS_WAKEUP_CFG_NONE	0
> +
>  /* WAKEUP STAT register bits */
>  #define USBSS_WAKEUP_STAT_OVERCURRENT	BIT(4)
>  #define USBSS_WAKEUP_STAT_LINESTATE	BIT(3)
> @@ -103,6 +110,7 @@ struct dwc3_data {
>  	struct regmap *syscon;
>  	unsigned int offset;
>  	unsigned int vbus_divider;
> +	u32 wakeup_stat;
>  };
>  
>  static const int dwc3_ti_rate_table[] = {	/* in KHZ */
> @@ -302,12 +310,17 @@ static int dwc3_ti_suspend_common(struct device *dev)
>  		/* Set wakeup config enable bits */
>  		reg = dwc3_ti_readl(data, USBSS_WAKEUP_CONFIG);
>  		if (current_prtcap_dir == DWC3_GCTL_PRTCAP_HOST) {
> -			reg |= USBSS_WAKEUP_CFG_LINESTATE_EN | USBSS_WAKEUP_CFG_OVERCURRENT_EN;
> +			reg = USBSS_WAKEUP_CFG_LINESTATE_EN | USBSS_WAKEUP_CFG_OVERCURRENT_EN;
>  		} else {
> -			reg |= USBSS_WAKEUP_CFG_OVERCURRENT_EN | USBSS_WAKEUP_CFG_LINESTATE_EN |
> -			       USBSS_WAKEUP_CFG_VBUSVALID_EN;
> +			reg = USBSS_WAKEUP_CFG_VBUSVALID_EN | USBSS_WAKEUP_CFG_SESSVALID_EN;
> +			/*
> +			 * Enable LINESTATE wake up only if connected to bus
> +			 * and in U2/L3 state else it causes spurious wake-up.
> +			 */
>  		}
>  		dwc3_ti_writel(data, USBSS_WAKEUP_CONFIG, reg);
> +		/* clear wakeup status so we know what caused the wake up */
> +		dwc3_ti_writel(data, USBSS_WAKEUP_STAT, USBSS_WAKEUP_STAT_CLR);
>  	}
>  
>  	clk_disable_unprepare(data->usb2_refclk);
> @@ -324,16 +337,11 @@ static int dwc3_ti_resume_common(struct device *dev)
>  
>  	if (device_may_wakeup(dev)) {
>  		/* Clear wakeup config enable bits */
> -		reg = dwc3_ti_readl(data, USBSS_WAKEUP_CONFIG);
> -		reg &= ~(USBSS_WAKEUP_CFG_OVERCURRENT_EN | USBSS_WAKEUP_CFG_LINESTATE_EN |
> -			 USBSS_WAKEUP_CFG_VBUSVALID_EN);
> -		dwc3_ti_writel(data, USBSS_WAKEUP_CONFIG, reg);
> +		dwc3_ti_writel(data, USBSS_WAKEUP_CONFIG, USBSS_WAKEUP_CFG_NONE);
>  	}
>  
>  	reg = dwc3_ti_readl(data, USBSS_WAKEUP_STAT);
> -	/* Clear the wakeup status with wakeup clear bit */
> -	reg |= USBSS_WAKEUP_STAT_CLR;
> -	dwc3_ti_writel(data, USBSS_WAKEUP_STAT, reg);
> +	data->wakeup_stat = reg;
>  
>  	return 0;
>  }
> -- 
> 2.34.1
> 

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

Thanks,
Thinh
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c
index 859b48279658..b22fb78bc8e7 100644
--- a/drivers/usb/dwc3/dwc3-am62.c
+++ b/drivers/usb/dwc3/dwc3-am62.c
@@ -60,6 +60,13 @@ 
 #define USBSS_WAKEUP_CFG_SESSVALID_EN	BIT(1)
 #define USBSS_WAKEUP_CFG_VBUSVALID_EN	BIT(0)
 
+#define USBSS_WAKEUP_CFG_ALL	(USBSS_WAKEUP_CFG_VBUSVALID_EN | \
+				 USBSS_WAKEUP_CFG_SESSVALID_EN | \
+				 USBSS_WAKEUP_CFG_LINESTATE_EN | \
+				 USBSS_WAKEUP_CFG_OVERCURRENT_EN)
+
+#define USBSS_WAKEUP_CFG_NONE	0
+
 /* WAKEUP STAT register bits */
 #define USBSS_WAKEUP_STAT_OVERCURRENT	BIT(4)
 #define USBSS_WAKEUP_STAT_LINESTATE	BIT(3)
@@ -103,6 +110,7 @@  struct dwc3_data {
 	struct regmap *syscon;
 	unsigned int offset;
 	unsigned int vbus_divider;
+	u32 wakeup_stat;
 };
 
 static const int dwc3_ti_rate_table[] = {	/* in KHZ */
@@ -302,12 +310,17 @@  static int dwc3_ti_suspend_common(struct device *dev)
 		/* Set wakeup config enable bits */
 		reg = dwc3_ti_readl(data, USBSS_WAKEUP_CONFIG);
 		if (current_prtcap_dir == DWC3_GCTL_PRTCAP_HOST) {
-			reg |= USBSS_WAKEUP_CFG_LINESTATE_EN | USBSS_WAKEUP_CFG_OVERCURRENT_EN;
+			reg = USBSS_WAKEUP_CFG_LINESTATE_EN | USBSS_WAKEUP_CFG_OVERCURRENT_EN;
 		} else {
-			reg |= USBSS_WAKEUP_CFG_OVERCURRENT_EN | USBSS_WAKEUP_CFG_LINESTATE_EN |
-			       USBSS_WAKEUP_CFG_VBUSVALID_EN;
+			reg = USBSS_WAKEUP_CFG_VBUSVALID_EN | USBSS_WAKEUP_CFG_SESSVALID_EN;
+			/*
+			 * Enable LINESTATE wake up only if connected to bus
+			 * and in U2/L3 state else it causes spurious wake-up.
+			 */
 		}
 		dwc3_ti_writel(data, USBSS_WAKEUP_CONFIG, reg);
+		/* clear wakeup status so we know what caused the wake up */
+		dwc3_ti_writel(data, USBSS_WAKEUP_STAT, USBSS_WAKEUP_STAT_CLR);
 	}
 
 	clk_disable_unprepare(data->usb2_refclk);
@@ -324,16 +337,11 @@  static int dwc3_ti_resume_common(struct device *dev)
 
 	if (device_may_wakeup(dev)) {
 		/* Clear wakeup config enable bits */
-		reg = dwc3_ti_readl(data, USBSS_WAKEUP_CONFIG);
-		reg &= ~(USBSS_WAKEUP_CFG_OVERCURRENT_EN | USBSS_WAKEUP_CFG_LINESTATE_EN |
-			 USBSS_WAKEUP_CFG_VBUSVALID_EN);
-		dwc3_ti_writel(data, USBSS_WAKEUP_CONFIG, reg);
+		dwc3_ti_writel(data, USBSS_WAKEUP_CONFIG, USBSS_WAKEUP_CFG_NONE);
 	}
 
 	reg = dwc3_ti_readl(data, USBSS_WAKEUP_STAT);
-	/* Clear the wakeup status with wakeup clear bit */
-	reg |= USBSS_WAKEUP_STAT_CLR;
-	dwc3_ti_writel(data, USBSS_WAKEUP_STAT, reg);
+	data->wakeup_stat = reg;
 
 	return 0;
 }