usb: dwc2: Problem with remote wakeup implementation

Commit Message

Maxim Devaev March 23, 2022, 10:24 p.m. UTC

Hello. I'm trying to implement remote wakeup signalling for dwc2.
The dwc3 driver in gadget mode has the ability to send a remote
wakeup signal to the host by writing 1 to the srp file:

echo 1 > /sys/class/udc/XXX/srp

My naive implementation was able to wake up the host on 5.10 kernel,
but it's not working anymore on 5.15, it's just does nothing now.
I tried to roll back all the changes in drivers/usb/dwc2 to the 5.10,
but it didn't help.

I don't have any DesignWare documentation, so I hope someone
can help me to make it work in the right way.

I will be very grateful.

Comments

Minas Harutyunyan April 4, 2022, 7:30 a.m. UTC | #1

Hi Maxim,

On 3/24/2022 2:24 AM, Maxim Devaev wrote:
> Hello. I'm trying to implement remote wakeup signalling for dwc2.
> The dwc3 driver in gadget mode has the ability to send a remote
> wakeup signal to the host by writing 1 to the srp file:
> 
> echo 1 > /sys/class/udc/XXX/srp
> 
> My naive implementation was able to wake up the host on 5.10 kernel,
> but it's not working anymore on 5.15, it's just does nothing now.
> I tried to roll back all the changes in drivers/usb/dwc2 to the 5.10,
> but it didn't help.
> 
> I don't have any DesignWare documentation, so I hope someone
> can help me to make it work in the right way.
> 

According databook RmtWkUpSig bit description below. You should also 
consider LPM state.

Remote Wakeup Signaling (RmtWkUpSig)
When the application sets this bit, the core initiates remote
signaling to wake up the USB host. The application must Set this bit
to instruct the core to exit the Suspend state. As specified in the
USB 2.0 specification, the application must clear this bit 1-15 ms
after setting it.
If LPM is enabled and the core is in the L1 (Sleep) state, when the
application sets this bit, the core initiates L1 remote signaling to
wake up the USB host. The application must set this bit to instruct
the core to exit the Sleep state. As specified in the LPM
specification, the hardware automatically clears this bit 50
microseconds (TL1DevDrvResume) after being set by the
application. The application must not set this bit when GLPMCFG
bRemoteWake from the previous LPM transaction is zero.

If it will not help, please check power status of core while resuming: 
hibernated, in partial power down, etc.

Thanks,
Minas

> I will be very grateful >
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 3146df6e6510..c64e573af7ca 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -4683,6 +4683,52 @@ static int dwc2_hsotg_vbus_draw(struct usb_gadget *gadget, unsigned int mA)
>   	return usb_phy_set_power(hsotg->uphy, mA);
>   }
>   
> +/**
> + * dwc2_hsotg_wakeup - send wakeup signal to the host
> + * @gadget: The usb gadget state
> + *
> + * If the gadget is in device mode and in the L1 or L2 state,
> + * it sends a wakeup signal to the host.
> + */
> +static int dwc2_hsotg_wakeup(struct usb_gadget *gadget)
> +{
> +	struct dwc2_hsotg *hsotg = to_hsotg(gadget);
> +	int ret = -1;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&hsotg->lock, flags);
> +
> +	if (!hsotg->remote_wakeup_allowed) {
> +		dev_dbg(hsotg->dev,
> +			"wakeup signalling skipped: is not allowed by host\n");
> +		goto skip;
> +	}
> +	if (hsotg->lx_state != DWC2_L1 && hsotg->lx_state != DWC2_L2) {
> +		dev_dbg(hsotg->dev,
> +			"wakeup signalling skipped: gadget not in L1/L2 state\n");
> +		goto skip;
> +	}
> +	if (!dwc2_is_device_mode(hsotg)) {
> +		dev_dbg(hsotg->dev,
> +			"wakeup signalling skipped: gadget not in device mode\n");
> +		goto skip;
> +	}
> +
> +	dev_dbg(hsotg->dev, "sending wakeup signal to the host");
> +
> +	dwc2_set_bit(hsotg, DCTL, DCTL_RMTWKUPSIG);
> +	mdelay(10);
> +	dwc2_clear_bit(hsotg, DCTL, DCTL_RMTWKUPSIG);
> +
> +	/* After the signalling, the USB core wakes up to L0 */
> +	hsotg->lx_state = DWC2_L0;
> +
> +	ret = 0;
> +skip:
> +	spin_unlock_irqrestore(&hsotg->lock, flags);
> +	return ret;
> +}
> +
>   static const struct usb_gadget_ops dwc2_hsotg_gadget_ops = {
>   	.get_frame	= dwc2_hsotg_gadget_getframe,
>   	.set_selfpowered	= dwc2_hsotg_set_selfpowered,
> @@ -4691,6 +4737,7 @@ static const struct usb_gadget_ops dwc2_hsotg_gadget_ops = {
>   	.pullup                 = dwc2_hsotg_pullup,
>   	.vbus_session		= dwc2_hsotg_vbus_session,
>   	.vbus_draw		= dwc2_hsotg_vbus_draw,
> +	.wakeup			= dwc2_hsotg_wakeup,
>   };
>   
>   /**
> 
>

Maxim Devaev April 6, 2022, 9:38 a.m. UTC | #2

В Mon, 4 Apr 2022 07:30:25 +0000
Minas Harutyunyan <[e-mail]> пишет:

> Hi Maxim,
> 
> According databook RmtWkUpSig bit description below. You should also 
> consider LPM state.
> 
> Remote Wakeup Signaling (RmtWkUpSig)
> When the application sets this bit, the core initiates remote
> signaling to wake up the USB host. The application must Set this bit
> to instruct the core to exit the Suspend state. As specified in the
> USB 2.0 specification, the application must clear this bit 1-15 ms
> after setting it.
> If LPM is enabled and the core is in the L1 (Sleep) state, when the
> application sets this bit, the core initiates L1 remote signaling to
> wake up the USB host. The application must set this bit to instruct
> the core to exit the Sleep state. As specified in the LPM
> specification, the hardware automatically clears this bit 50
> microseconds (TL1DevDrvResume) after being set by the
> application. The application must not set this bit when GLPMCFG
> bRemoteWake from the previous LPM transaction is zero.
> 
> If it will not help, please check power status of core while resuming: 
> hibernated, in partial power down, etc.
> 
> Thanks,
> Minas
> 

Thank you so much! This is very useful.
I fixed the problem, it was related with the new clock gating code.

I'll try to make a clear patch and submit it to the kernel.

Patch

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 3146df6e6510..c64e573af7ca 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -4683,6 +4683,52 @@  static int dwc2_hsotg_vbus_draw(struct usb_gadget *gadget, unsigned int mA)
 	return usb_phy_set_power(hsotg->uphy, mA);
 }
 
+/**
+ * dwc2_hsotg_wakeup - send wakeup signal to the host
+ * @gadget: The usb gadget state
+ *
+ * If the gadget is in device mode and in the L1 or L2 state,
+ * it sends a wakeup signal to the host.
+ */
+static int dwc2_hsotg_wakeup(struct usb_gadget *gadget)
+{
+	struct dwc2_hsotg *hsotg = to_hsotg(gadget);
+	int ret = -1;
+	unsigned long flags;
+
+	spin_lock_irqsave(&hsotg->lock, flags);
+
+	if (!hsotg->remote_wakeup_allowed) {
+		dev_dbg(hsotg->dev,
+			"wakeup signalling skipped: is not allowed by host\n");
+		goto skip;
+	}
+	if (hsotg->lx_state != DWC2_L1 && hsotg->lx_state != DWC2_L2) {
+		dev_dbg(hsotg->dev,
+			"wakeup signalling skipped: gadget not in L1/L2 state\n");
+		goto skip;
+	}
+	if (!dwc2_is_device_mode(hsotg)) {
+		dev_dbg(hsotg->dev,
+			"wakeup signalling skipped: gadget not in device mode\n");
+		goto skip;
+	}
+
+	dev_dbg(hsotg->dev, "sending wakeup signal to the host");
+
+	dwc2_set_bit(hsotg, DCTL, DCTL_RMTWKUPSIG);
+	mdelay(10);
+	dwc2_clear_bit(hsotg, DCTL, DCTL_RMTWKUPSIG);
+
+	/* After the signalling, the USB core wakes up to L0 */
+	hsotg->lx_state = DWC2_L0;
+
+	ret = 0;
+skip:
+	spin_unlock_irqrestore(&hsotg->lock, flags);
+	return ret;
+}
+
 static const struct usb_gadget_ops dwc2_hsotg_gadget_ops = {
 	.get_frame	= dwc2_hsotg_gadget_getframe,
 	.set_selfpowered	= dwc2_hsotg_set_selfpowered,
@@ -4691,6 +4737,7 @@  static const struct usb_gadget_ops dwc2_hsotg_gadget_ops = {
 	.pullup                 = dwc2_hsotg_pullup,
 	.vbus_session		= dwc2_hsotg_vbus_session,
 	.vbus_draw		= dwc2_hsotg_vbus_draw,
+	.wakeup			= dwc2_hsotg_wakeup,
 };
 
 /**