[v1] usb: gadget: zero: add function wakeup support

[PATCH] usb: gadget: zero: add function wakeup support

Posted by Xu Yang 2 weeks, 3 days ago

When the device working at enhanced superspeed, it needs to send function
remote wakeup signal to the host instead of device remote wakeup. Add
function wakeup support for the purpose.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/gadget/legacy/zero.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/legacy/zero.c b/drivers/usb/gadget/legacy/zero.c
index a05785bdeb30..fe286b597f9f 100644
--- a/drivers/usb/gadget/legacy/zero.c
+++ b/drivers/usb/gadget/legacy/zero.c
@@ -147,6 +147,12 @@ static struct usb_gadget_strings *dev_strings[] = {
 	NULL,
 };
 
+static struct usb_function *func_lb;
+static struct usb_function_instance *func_inst_lb;
+
+static struct usb_function *func_ss;
+static struct usb_function_instance *func_inst_ss;
+
 /*-------------------------------------------------------------------------*/
 
 static struct timer_list	autoresume_timer;
@@ -156,6 +162,7 @@ static void zero_autoresume(struct timer_list *unused)
 {
 	struct usb_composite_dev	*cdev = autoresume_cdev;
 	struct usb_gadget		*g = cdev->gadget;
+	int status;
 
 	/* unconfigured devices can't issue wakeups */
 	if (!cdev->config)
@@ -165,10 +172,18 @@ static void zero_autoresume(struct timer_list *unused)
 	 * more significant than just a timer firing; likely
 	 * because of some direct user request.
 	 */
-	if (g->speed != USB_SPEED_UNKNOWN) {
-		int status = usb_gadget_wakeup(g);
-		INFO(cdev, "%s --> %d\n", __func__, status);
+	if (g->speed == USB_SPEED_UNKNOWN)
+		return;
+
+	if (g->speed >= USB_SPEED_SUPER) {
+		if (loopdefault)
+			status = usb_func_wakeup(func_lb);
+		else
+			status = usb_func_wakeup(func_ss);
+	} else {
+		status = usb_gadget_wakeup(g);
 	}
+	INFO(cdev, "%s --> %d\n", __func__, status);
 }
 
 static void zero_suspend(struct usb_composite_dev *cdev)
@@ -206,9 +221,6 @@ static struct usb_configuration loopback_driver = {
 	/* .iConfiguration = DYNAMIC */
 };
 
-static struct usb_function *func_ss;
-static struct usb_function_instance *func_inst_ss;
-
 static int ss_config_setup(struct usb_configuration *c,
 		const struct usb_ctrlrequest *ctrl)
 {
@@ -248,9 +260,6 @@ module_param_named(isoc_maxburst, gzero_options.isoc_maxburst, uint,
 		S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(isoc_maxburst, "0 - 15 (ss only)");
 
-static struct usb_function *func_lb;
-static struct usb_function_instance *func_inst_lb;
-
 module_param_named(qlen, gzero_options.qlen, uint, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(qlen, "depth of loopback queue");
 
-- 
2.34.1

Re: [PATCH] usb: gadget: zero: add function wakeup support

Posted by Alan Stern 2 weeks, 3 days ago

On Mon, Sep 15, 2025 at 05:02:30PM +0800, Xu Yang wrote:
> When the device working at enhanced superspeed, it needs to send function

Is this also true when the device is connected at regular (not enhanced) 
SuperSpeed?

> remote wakeup signal to the host instead of device remote wakeup. Add
> function wakeup support for the purpose.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/gadget/legacy/zero.c | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/gadget/legacy/zero.c b/drivers/usb/gadget/legacy/zero.c
> index a05785bdeb30..fe286b597f9f 100644
> --- a/drivers/usb/gadget/legacy/zero.c
> +++ b/drivers/usb/gadget/legacy/zero.c
> @@ -147,6 +147,12 @@ static struct usb_gadget_strings *dev_strings[] = {
>  	NULL,
>  };
>  
> +static struct usb_function *func_lb;
> +static struct usb_function_instance *func_inst_lb;
> +
> +static struct usb_function *func_ss;
> +static struct usb_function_instance *func_inst_ss;
> +
>  /*-------------------------------------------------------------------------*/
>  
>  static struct timer_list	autoresume_timer;
> @@ -156,6 +162,7 @@ static void zero_autoresume(struct timer_list *unused)
>  {
>  	struct usb_composite_dev	*cdev = autoresume_cdev;
>  	struct usb_gadget		*g = cdev->gadget;
> +	int status;

Please use the same style for your new code as for the existing code.  
In this case, use tabs to align "status" with "*g" and "*cdev".

Alan Stern

Re: [PATCH] usb: gadget: zero: add function wakeup support

Posted by Xu Yang 2 weeks, 2 days ago

On Mon, Sep 15, 2025 at 10:05:15AM -0400, Alan Stern wrote:
> On Mon, Sep 15, 2025 at 05:02:30PM +0800, Xu Yang wrote:
> > When the device working at enhanced superspeed, it needs to send function
> 
> Is this also true when the device is connected at regular (not enhanced) 
> SuperSpeed?

Yes, it's true. USB3.2 Spec regards it as SuperSpeed/SuperSpeedPlus.

Enhanced SuperSpeed | An adjective referring to any valid collection of USB
                      defined features defined for the bus that runs over the
		      SSRx and SSTx differential pairs in a USB 3.x system.
		      It is used in place of phrases like SuperSpeed/SuperSpeedPlus.

> 
> > remote wakeup signal to the host instead of device remote wakeup. Add
> > function wakeup support for the purpose.
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  drivers/usb/gadget/legacy/zero.c | 27 ++++++++++++++++++---------
> >  1 file changed, 18 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/usb/gadget/legacy/zero.c b/drivers/usb/gadget/legacy/zero.c
> > index a05785bdeb30..fe286b597f9f 100644
> > --- a/drivers/usb/gadget/legacy/zero.c
> > +++ b/drivers/usb/gadget/legacy/zero.c
> > @@ -147,6 +147,12 @@ static struct usb_gadget_strings *dev_strings[] = {
> >  	NULL,
> >  };
> >  
> > +static struct usb_function *func_lb;
> > +static struct usb_function_instance *func_inst_lb;
> > +
> > +static struct usb_function *func_ss;
> > +static struct usb_function_instance *func_inst_ss;
> > +
> >  /*-------------------------------------------------------------------------*/
> >  
> >  static struct timer_list	autoresume_timer;
> > @@ -156,6 +162,7 @@ static void zero_autoresume(struct timer_list *unused)
> >  {
> >  	struct usb_composite_dev	*cdev = autoresume_cdev;
> >  	struct usb_gadget		*g = cdev->gadget;
> > +	int status;
> 
> Please use the same style for your new code as for the existing code.  
> In this case, use tabs to align "status" with "*g" and "*cdev".

My bad. I will fix it. 
Surprisingly, checkpatch.pl didn't report this issue. But it always report
a line break that's not properly aligned with the parentheses. :)

Thanks,
Xu Yang

> 
> Alan Stern

Re: [PATCH] usb: gadget: zero: add function wakeup support

Posted by Greg KH 2 weeks, 3 days ago

On Mon, Sep 15, 2025 at 05:02:30PM +0800, Xu Yang wrote:
> When the device working at enhanced superspeed, it needs to send function
> remote wakeup signal to the host instead of device remote wakeup. Add
> function wakeup support for the purpose.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/gadget/legacy/zero.c | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/gadget/legacy/zero.c b/drivers/usb/gadget/legacy/zero.c
> index a05785bdeb30..fe286b597f9f 100644
> --- a/drivers/usb/gadget/legacy/zero.c
> +++ b/drivers/usb/gadget/legacy/zero.c
> @@ -147,6 +147,12 @@ static struct usb_gadget_strings *dev_strings[] = {
>  	NULL,
>  };
>  
> +static struct usb_function *func_lb;
> +static struct usb_function_instance *func_inst_lb;
> +
> +static struct usb_function *func_ss;
> +static struct usb_function_instance *func_inst_ss;
> +
>  /*-------------------------------------------------------------------------*/
>  
>  static struct timer_list	autoresume_timer;
> @@ -156,6 +162,7 @@ static void zero_autoresume(struct timer_list *unused)
>  {
>  	struct usb_composite_dev	*cdev = autoresume_cdev;
>  	struct usb_gadget		*g = cdev->gadget;
> +	int status;
>  
>  	/* unconfigured devices can't issue wakeups */
>  	if (!cdev->config)
> @@ -165,10 +172,18 @@ static void zero_autoresume(struct timer_list *unused)
>  	 * more significant than just a timer firing; likely
>  	 * because of some direct user request.
>  	 */
> -	if (g->speed != USB_SPEED_UNKNOWN) {
> -		int status = usb_gadget_wakeup(g);
> -		INFO(cdev, "%s --> %d\n", __func__, status);
> +	if (g->speed == USB_SPEED_UNKNOWN)
> +		return;
> +
> +	if (g->speed >= USB_SPEED_SUPER) {
> +		if (loopdefault)
> +			status = usb_func_wakeup(func_lb);
> +		else
> +			status = usb_func_wakeup(func_ss);
> +	} else {
> +		status = usb_gadget_wakeup(g);
>  	}
> +	INFO(cdev, "%s --> %d\n", __func__, status);

Is this INFO() call still needed?  Shouldn't that be removed?

thanks,

greg k-h

Re: [PATCH] usb: gadget: zero: add function wakeup support

Posted by Xu Yang 2 weeks, 2 days ago

On Mon, Sep 15, 2025 at 01:36:07PM +0200, Greg KH wrote:
> On Mon, Sep 15, 2025 at 05:02:30PM +0800, Xu Yang wrote:
> > When the device working at enhanced superspeed, it needs to send function
> > remote wakeup signal to the host instead of device remote wakeup. Add
> > function wakeup support for the purpose.
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  drivers/usb/gadget/legacy/zero.c | 27 ++++++++++++++++++---------
> >  1 file changed, 18 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/usb/gadget/legacy/zero.c b/drivers/usb/gadget/legacy/zero.c
> > index a05785bdeb30..fe286b597f9f 100644
> > --- a/drivers/usb/gadget/legacy/zero.c
> > +++ b/drivers/usb/gadget/legacy/zero.c
> > @@ -147,6 +147,12 @@ static struct usb_gadget_strings *dev_strings[] = {
> >  	NULL,
> >  };
> >  
> > +static struct usb_function *func_lb;
> > +static struct usb_function_instance *func_inst_lb;
> > +
> > +static struct usb_function *func_ss;
> > +static struct usb_function_instance *func_inst_ss;
> > +
> >  /*-------------------------------------------------------------------------*/
> >  
> >  static struct timer_list	autoresume_timer;
> > @@ -156,6 +162,7 @@ static void zero_autoresume(struct timer_list *unused)
> >  {
> >  	struct usb_composite_dev	*cdev = autoresume_cdev;
> >  	struct usb_gadget		*g = cdev->gadget;
> > +	int status;
> >  
> >  	/* unconfigured devices can't issue wakeups */
> >  	if (!cdev->config)
> > @@ -165,10 +172,18 @@ static void zero_autoresume(struct timer_list *unused)
> >  	 * more significant than just a timer firing; likely
> >  	 * because of some direct user request.
> >  	 */
> > -	if (g->speed != USB_SPEED_UNKNOWN) {
> > -		int status = usb_gadget_wakeup(g);
> > -		INFO(cdev, "%s --> %d\n", __func__, status);
> > +	if (g->speed == USB_SPEED_UNKNOWN)
> > +		return;
> > +
> > +	if (g->speed >= USB_SPEED_SUPER) {
> > +		if (loopdefault)
> > +			status = usb_func_wakeup(func_lb);
> > +		else
> > +			status = usb_func_wakeup(func_ss);
> > +	} else {
> > +		status = usb_gadget_wakeup(g);
> >  	}
> > +	INFO(cdev, "%s --> %d\n", __func__, status);
> 
> Is this INFO() call still needed?  Shouldn't that be removed?

For me, this information is useful to know the test results. I prefer
to keep it.

Thanks,
Xu Yang

> 
> thanks,
> 
> greg k-h