diff mbox series

[v2,4/5] usb: dwc3: Add function suspend and function wakeup support

Message ID 1673992507-7823-5-git-send-email-quic_eserrao@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Add function suspend/resume and remote wakeup support | expand

Commit Message

Elson Roy Serrao Jan. 17, 2023, 9:55 p.m. UTC 
USB host sends function suspend and function resume notifications to
the interface through SET_FEATURE/CLEAR_FEATURE setup packets.
Add support to handle these packets by delegating the requests to
composite layer. Also add support to handle function wake notification
requests to exit from function suspend state.

Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
---
 drivers/usb/dwc3/core.h   |  1 +
 drivers/usb/dwc3/debug.h  |  2 ++
 drivers/usb/dwc3/ep0.c    | 12 ++++--------
 drivers/usb/dwc3/gadget.c | 36 +++++++++++++++++++++++++++++++++++-
 4 files changed, 42 insertions(+), 9 deletions(-)

Comments

Thinh Nguyen Jan. 19, 2023, 2:18 a.m. UTC | #1 
On Tue, Jan 17, 2023, Elson Roy Serrao wrote:
> USB host sends function suspend and function resume notifications to
> the interface through SET_FEATURE/CLEAR_FEATURE setup packets.
> Add support to handle these packets by delegating the requests to
> composite layer. Also add support to handle function wake notification
> requests to exit from function suspend state.
> 
> Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
> ---
>  drivers/usb/dwc3/core.h   |  1 +
>  drivers/usb/dwc3/debug.h  |  2 ++
>  drivers/usb/dwc3/ep0.c    | 12 ++++--------
>  drivers/usb/dwc3/gadget.c | 36 +++++++++++++++++++++++++++++++++++-
>  4 files changed, 42 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 53ded08..5cda645 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -526,6 +526,7 @@
>  #define DWC3_DGCMD_SET_ENDPOINT_NRDY	0x0c
>  #define DWC3_DGCMD_SET_ENDPOINT_PRIME	0x0d
>  #define DWC3_DGCMD_RUN_SOC_BUS_LOOPBACK	0x10
> +#define DWC3_DGCMD_DEV_NOTIFICATION	0x07
>  
>  #define DWC3_DGCMD_STATUS(n)		(((n) >> 12) & 0x0F)
>  #define DWC3_DGCMD_CMDACT		BIT(10)
> diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
> index 48b44b8..0897d9d 100644
> --- a/drivers/usb/dwc3/debug.h
> +++ b/drivers/usb/dwc3/debug.h
> @@ -72,6 +72,8 @@ dwc3_gadget_generic_cmd_string(u8 cmd)
>  		return "Set Endpoint Prime";
>  	case DWC3_DGCMD_RUN_SOC_BUS_LOOPBACK:
>  		return "Run SoC Bus Loopback Test";
> +	case DWC3_DGCMD_DEV_NOTIFICATION:
> +		return "Device Notification";
>  	default:
>  		return "UNKNOWN";
>  	}
> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index 0c1203d..e05c2b9 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
> @@ -30,6 +30,8 @@
>  static void __dwc3_ep0_do_control_status(struct dwc3 *dwc, struct dwc3_ep *dep);
>  static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
>  		struct dwc3_ep *dep, struct dwc3_request *req);
> +static int dwc3_ep0_delegate_req(struct dwc3 *dwc,
> +				 struct usb_ctrlrequest *ctrl);
>  
>  static void dwc3_ep0_prepare_one_trb(struct dwc3_ep *dep,
>  		dma_addr_t buf_dma, u32 len, u32 type, bool chain)
> @@ -367,7 +369,7 @@ static int dwc3_ep0_handle_status(struct dwc3 *dwc,
>  		 * Function Remote Wake Capable	D0
>  		 * Function Remote Wakeup	D1
>  		 */
> -		break;
> +		return dwc3_ep0_delegate_req(dwc, ctrl);
>  
>  	case USB_RECIP_ENDPOINT:
>  		dep = dwc3_wIndex_to_dep(dwc, ctrl->wIndex);
> @@ -514,13 +516,7 @@ static int dwc3_ep0_handle_intf(struct dwc3 *dwc,
>  
>  	switch (wValue) {
>  	case USB_INTRF_FUNC_SUSPEND:
> -		/*
> -		 * REVISIT: Ideally we would enable some low power mode here,
> -		 * however it's unclear what we should be doing here.
> -		 *
> -		 * For now, we're not doing anything, just making sure we return
> -		 * 0 so USB Command Verifier tests pass without any errors.
> -		 */
> +		ret = dwc3_ep0_delegate_req(dwc, ctrl);
>  		break;
>  	default:
>  		ret = -EINVAL;
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index db4b438..1c958c4 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2346,7 +2346,7 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc, bool async)
>  		return 0;
>  
>  	/* poll until Link State changes to ON */
> -	retries = 20000;
> +	retries = 30000;

This seems like a fix that needs to be splitted to a separate change.
read_poll_timeout_atomic() would be useful here (as a separate/unrelated
change for status polling).

>  
>  	while (retries--) {
>  		reg = dwc3_readl(dwc->regs, DWC3_DSTS);
> @@ -2383,6 +2383,39 @@ static int dwc3_gadget_wakeup(struct usb_gadget *g)
>  	return ret;
>  }
>  
> +static void dwc3_resume_gadget(struct dwc3 *dwc);
> +
> +static int dwc3_gadget_func_wakeup(struct usb_gadget *g, int interface_id)
> +{
> +	struct  dwc3		*dwc = gadget_to_dwc(g);
> +	unsigned long		flags;
> +	int			ret;
> +
> +	spin_lock_irqsave(&dwc->lock, flags);
> +	/*
> +	 * If the link is in LPM, first bring the link to U0

Reword LMP -> low power.

> +	 * before triggering function remote wakeup.
> +	 */
> +	if (dwc->link_state == DWC3_LINK_STATE_U3) {
> +		ret = __dwc3_gadget_wakeup(dwc, false);
> +		if (ret) {
> +			spin_unlock_irqrestore(&dwc->lock, flags);
> +			return -EINVAL;
> +		}
> +		dwc3_resume_gadget(dwc);
> +		dwc->link_state = DWC3_LINK_STATE_U0;
> +	}
> +
> +	ret = dwc3_send_gadget_generic_command(dwc, DWC3_DGCMD_DEV_NOTIFICATION,
> +					       0x1 | (interface_id << 4));

Create a macro for 0x1. Something like DWC3_DGCMDPAR_DN_FUNC_WAKE. Also,
create an macro for interface selection. Maybe
DWC3_DGCMDPAR_INTF_SEL(interface_id).

> +	if (ret)
> +		dev_err(dwc->dev, "function remote wakeup failed, ret:%d\n", ret);
> +
> +	spin_unlock_irqrestore(&dwc->lock, flags);
> +
> +	return ret;
> +}
> +
>  static int dwc3_gadget_set_selfpowered(struct usb_gadget *g,
>  		int is_selfpowered)
>  {
> @@ -3012,6 +3045,7 @@ static void dwc3_gadget_async_callbacks(struct usb_gadget *g, bool enable)
>  static const struct usb_gadget_ops dwc3_gadget_ops = {
>  	.get_frame		= dwc3_gadget_get_frame,
>  	.wakeup			= dwc3_gadget_wakeup,
> +	.func_wakeup		= dwc3_gadget_func_wakeup,
>  	.set_selfpowered	= dwc3_gadget_set_selfpowered,
>  	.pullup			= dwc3_gadget_pullup,
>  	.udc_start		= dwc3_gadget_start,
> -- 
> 2.7.4
> 

Thanks,
Thinh
Elson Roy Serrao Jan. 19, 2023, 8:03 p.m. UTC | #2 
On 1/18/2023 6:18 PM, Thinh Nguyen wrote:
> On Tue, Jan 17, 2023, Elson Roy Serrao wrote:
>> USB host sends function suspend and function resume notifications to
>> the interface through SET_FEATURE/CLEAR_FEATURE setup packets.
>> Add support to handle these packets by delegating the requests to
>> composite layer. Also add support to handle function wake notification
>> requests to exit from function suspend state.
>>
>> Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
>> ---
>>   drivers/usb/dwc3/core.h   |  1 +
>>   drivers/usb/dwc3/debug.h  |  2 ++
>>   drivers/usb/dwc3/ep0.c    | 12 ++++--------
>>   drivers/usb/dwc3/gadget.c | 36 +++++++++++++++++++++++++++++++++++-
>>   4 files changed, 42 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 53ded08..5cda645 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -526,6 +526,7 @@
>>   #define DWC3_DGCMD_SET_ENDPOINT_NRDY	0x0c
>>   #define DWC3_DGCMD_SET_ENDPOINT_PRIME	0x0d
>>   #define DWC3_DGCMD_RUN_SOC_BUS_LOOPBACK	0x10
>> +#define DWC3_DGCMD_DEV_NOTIFICATION	0x07
>>   
>>   #define DWC3_DGCMD_STATUS(n)		(((n) >> 12) & 0x0F)
>>   #define DWC3_DGCMD_CMDACT		BIT(10)
>> diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
>> index 48b44b8..0897d9d 100644
>> --- a/drivers/usb/dwc3/debug.h
>> +++ b/drivers/usb/dwc3/debug.h
>> @@ -72,6 +72,8 @@ dwc3_gadget_generic_cmd_string(u8 cmd)
>>   		return "Set Endpoint Prime";
>>   	case DWC3_DGCMD_RUN_SOC_BUS_LOOPBACK:
>>   		return "Run SoC Bus Loopback Test";
>> +	case DWC3_DGCMD_DEV_NOTIFICATION:
>> +		return "Device Notification";
>>   	default:
>>   		return "UNKNOWN";
>>   	}
>> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
>> index 0c1203d..e05c2b9 100644
>> --- a/drivers/usb/dwc3/ep0.c
>> +++ b/drivers/usb/dwc3/ep0.c
>> @@ -30,6 +30,8 @@
>>   static void __dwc3_ep0_do_control_status(struct dwc3 *dwc, struct dwc3_ep *dep);
>>   static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
>>   		struct dwc3_ep *dep, struct dwc3_request *req);
>> +static int dwc3_ep0_delegate_req(struct dwc3 *dwc,
>> +				 struct usb_ctrlrequest *ctrl);
>>   
>>   static void dwc3_ep0_prepare_one_trb(struct dwc3_ep *dep,
>>   		dma_addr_t buf_dma, u32 len, u32 type, bool chain)
>> @@ -367,7 +369,7 @@ static int dwc3_ep0_handle_status(struct dwc3 *dwc,
>>   		 * Function Remote Wake Capable	D0
>>   		 * Function Remote Wakeup	D1
>>   		 */
>> -		break;
>> +		return dwc3_ep0_delegate_req(dwc, ctrl);
>>   
>>   	case USB_RECIP_ENDPOINT:
>>   		dep = dwc3_wIndex_to_dep(dwc, ctrl->wIndex);
>> @@ -514,13 +516,7 @@ static int dwc3_ep0_handle_intf(struct dwc3 *dwc,
>>   
>>   	switch (wValue) {
>>   	case USB_INTRF_FUNC_SUSPEND:
>> -		/*
>> -		 * REVISIT: Ideally we would enable some low power mode here,
>> -		 * however it's unclear what we should be doing here.
>> -		 *
>> -		 * For now, we're not doing anything, just making sure we return
>> -		 * 0 so USB Command Verifier tests pass without any errors.
>> -		 */
>> +		ret = dwc3_ep0_delegate_req(dwc, ctrl);
>>   		break;
>>   	default:
>>   		ret = -EINVAL;
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index db4b438..1c958c4 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2346,7 +2346,7 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc, bool async)
>>   		return 0;
>>   
>>   	/* poll until Link State changes to ON */
>> -	retries = 20000;
>> +	retries = 30000;
> 
> This seems like a fix that needs to be splitted to a separate change.
> read_poll_timeout_atomic() would be useful here (as a separate/unrelated
> change for status polling).
> 
Done.

>>   
>>   	while (retries--) {
>>   		reg = dwc3_readl(dwc->regs, DWC3_DSTS);
>> @@ -2383,6 +2383,39 @@ static int dwc3_gadget_wakeup(struct usb_gadget *g)
>>   	return ret;
>>   }
>>   
>> +static void dwc3_resume_gadget(struct dwc3 *dwc);
>> +
>> +static int dwc3_gadget_func_wakeup(struct usb_gadget *g, int interface_id)
>> +{
>> +	struct  dwc3		*dwc = gadget_to_dwc(g);
>> +	unsigned long		flags;
>> +	int			ret;
>> +
>> +	spin_lock_irqsave(&dwc->lock, flags);
>> +	/*
>> +	 * If the link is in LPM, first bring the link to U0
> 
> Reword LMP -> low power.
> 
Sure.

>> +	 * before triggering function remote wakeup.
>> +	 */
>> +	if (dwc->link_state == DWC3_LINK_STATE_U3) {
>> +		ret = __dwc3_gadget_wakeup(dwc, false);
>> +		if (ret) {
>> +			spin_unlock_irqrestore(&dwc->lock, flags);
>> +			return -EINVAL;
>> +		}
>> +		dwc3_resume_gadget(dwc);
>> +		dwc->link_state = DWC3_LINK_STATE_U0;
>> +	}
>> +
>> +	ret = dwc3_send_gadget_generic_command(dwc, DWC3_DGCMD_DEV_NOTIFICATION,
>> +					       0x1 | (interface_id << 4));
> 
> Create a macro for 0x1. Something like DWC3_DGCMDPAR_DN_FUNC_WAKE. Also,
> create an macro for interface selection. Maybe
> DWC3_DGCMDPAR_INTF_SEL(interface_id).
> 
Sure.

>> +	if (ret)
>> +		dev_err(dwc->dev, "function remote wakeup failed, ret:%d\n", ret);
>> +
>> +	spin_unlock_irqrestore(&dwc->lock, flags);
>> +
>> +	return ret;
>> +}
>> +
>>   static int dwc3_gadget_set_selfpowered(struct usb_gadget *g,
>>   		int is_selfpowered)
>>   {
>> @@ -3012,6 +3045,7 @@ static void dwc3_gadget_async_callbacks(struct usb_gadget *g, bool enable)
>>   static const struct usb_gadget_ops dwc3_gadget_ops = {
>>   	.get_frame		= dwc3_gadget_get_frame,
>>   	.wakeup			= dwc3_gadget_wakeup,
>> +	.func_wakeup		= dwc3_gadget_func_wakeup,
>>   	.set_selfpowered	= dwc3_gadget_set_selfpowered,
>>   	.pullup			= dwc3_gadget_pullup,
>>   	.udc_start		= dwc3_gadget_start,
>> -- 
>> 2.7.4
>>
> 
> Thanks,
> Thinh
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 53ded08..5cda645 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -526,6 +526,7 @@ 
 #define DWC3_DGCMD_SET_ENDPOINT_NRDY	0x0c
 #define DWC3_DGCMD_SET_ENDPOINT_PRIME	0x0d
 #define DWC3_DGCMD_RUN_SOC_BUS_LOOPBACK	0x10
+#define DWC3_DGCMD_DEV_NOTIFICATION	0x07
 
 #define DWC3_DGCMD_STATUS(n)		(((n) >> 12) & 0x0F)
 #define DWC3_DGCMD_CMDACT		BIT(10)
diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
index 48b44b8..0897d9d 100644
--- a/drivers/usb/dwc3/debug.h
+++ b/drivers/usb/dwc3/debug.h
@@ -72,6 +72,8 @@  dwc3_gadget_generic_cmd_string(u8 cmd)
 		return "Set Endpoint Prime";
 	case DWC3_DGCMD_RUN_SOC_BUS_LOOPBACK:
 		return "Run SoC Bus Loopback Test";
+	case DWC3_DGCMD_DEV_NOTIFICATION:
+		return "Device Notification";
 	default:
 		return "UNKNOWN";
 	}
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 0c1203d..e05c2b9 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -30,6 +30,8 @@ 
 static void __dwc3_ep0_do_control_status(struct dwc3 *dwc, struct dwc3_ep *dep);
 static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
 		struct dwc3_ep *dep, struct dwc3_request *req);
+static int dwc3_ep0_delegate_req(struct dwc3 *dwc,
+				 struct usb_ctrlrequest *ctrl);
 
 static void dwc3_ep0_prepare_one_trb(struct dwc3_ep *dep,
 		dma_addr_t buf_dma, u32 len, u32 type, bool chain)
@@ -367,7 +369,7 @@  static int dwc3_ep0_handle_status(struct dwc3 *dwc,
 		 * Function Remote Wake Capable	D0
 		 * Function Remote Wakeup	D1
 		 */
-		break;
+		return dwc3_ep0_delegate_req(dwc, ctrl);
 
 	case USB_RECIP_ENDPOINT:
 		dep = dwc3_wIndex_to_dep(dwc, ctrl->wIndex);
@@ -514,13 +516,7 @@  static int dwc3_ep0_handle_intf(struct dwc3 *dwc,
 
 	switch (wValue) {
 	case USB_INTRF_FUNC_SUSPEND:
-		/*
-		 * REVISIT: Ideally we would enable some low power mode here,
-		 * however it's unclear what we should be doing here.
-		 *
-		 * For now, we're not doing anything, just making sure we return
-		 * 0 so USB Command Verifier tests pass without any errors.
-		 */
+		ret = dwc3_ep0_delegate_req(dwc, ctrl);
 		break;
 	default:
 		ret = -EINVAL;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index db4b438..1c958c4 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2346,7 +2346,7 @@  static int __dwc3_gadget_wakeup(struct dwc3 *dwc, bool async)
 		return 0;
 
 	/* poll until Link State changes to ON */
-	retries = 20000;
+	retries = 30000;
 
 	while (retries--) {
 		reg = dwc3_readl(dwc->regs, DWC3_DSTS);
@@ -2383,6 +2383,39 @@  static int dwc3_gadget_wakeup(struct usb_gadget *g)
 	return ret;
 }
 
+static void dwc3_resume_gadget(struct dwc3 *dwc);
+
+static int dwc3_gadget_func_wakeup(struct usb_gadget *g, int interface_id)
+{
+	struct  dwc3		*dwc = gadget_to_dwc(g);
+	unsigned long		flags;
+	int			ret;
+
+	spin_lock_irqsave(&dwc->lock, flags);
+	/*
+	 * If the link is in LPM, first bring the link to U0
+	 * before triggering function remote wakeup.
+	 */
+	if (dwc->link_state == DWC3_LINK_STATE_U3) {
+		ret = __dwc3_gadget_wakeup(dwc, false);
+		if (ret) {
+			spin_unlock_irqrestore(&dwc->lock, flags);
+			return -EINVAL;
+		}
+		dwc3_resume_gadget(dwc);
+		dwc->link_state = DWC3_LINK_STATE_U0;
+	}
+
+	ret = dwc3_send_gadget_generic_command(dwc, DWC3_DGCMD_DEV_NOTIFICATION,
+					       0x1 | (interface_id << 4));
+	if (ret)
+		dev_err(dwc->dev, "function remote wakeup failed, ret:%d\n", ret);
+
+	spin_unlock_irqrestore(&dwc->lock, flags);
+
+	return ret;
+}
+
 static int dwc3_gadget_set_selfpowered(struct usb_gadget *g,
 		int is_selfpowered)
 {
@@ -3012,6 +3045,7 @@  static void dwc3_gadget_async_callbacks(struct usb_gadget *g, bool enable)
 static const struct usb_gadget_ops dwc3_gadget_ops = {
 	.get_frame		= dwc3_gadget_get_frame,
 	.wakeup			= dwc3_gadget_wakeup,
+	.func_wakeup		= dwc3_gadget_func_wakeup,
 	.set_selfpowered	= dwc3_gadget_set_selfpowered,
 	.pullup			= dwc3_gadget_pullup,
 	.udc_start		= dwc3_gadget_start,