Re: USB Gadget EEM Suspend/Resume

[Peter Chen <peter.chen@nxp.com>]

On 20-11-16 23:11:20, Neuenschwander, Bowe wrote:
> Hello,
> 
> I was hoping for some input  on how USB Ethernet Gadget drivers should handle USB suspend/resume  events.  We have a case where the USB suspend hook is being called for  an EEM Gadget; but since that hook is currently unimplemented, the interface remains in an active/enabled state.  I have  attached a patch that seems to fix this issue for EEM devices by  calling gether_disconnect() on suspend and gether_connect() on resume;  but I do not if this is actually correct behavior, so I was looking for some advice on that.  It seems that since the gadget driver cannot send data until the USB host resumes the USB link,  that the interface should be considered disconnected.
> 

Please wrap up to 80 characters per line, that's easy for reading.
Would you please describe the real issue you have met? I have a simply
ping test for EEM, it could work after resume, the packet is pending
if host is suspended.


64 bytes from 192.168.1.1: icmp_seq=154 ttl=64 time=0.358 ms
64 bytes from 192.168.1.1: icmp_seq=155 ttl=64 time=0.375 ms
64 bytes from 192.168.1.1: icmp_seq=156 ttl=64 time=0.364 ms
64 bytes from 192.168.1.1: icmp_seq=157 ttl=64 time=0.359 ms
64 bytes from 192.168.1.1: icmp_seq=158 ttl=64 time=3223 ms
64 bytes from 192.168.1.1: icmp_seq=159 ttl=64 time=2199 ms
64 bytes from 192.168.1.1: icmp_seq=160 ttl=64 time=1175 ms
64 bytes from 192.168.1.1: icmp_seq=161 ttl=64 time=151 ms
64 bytes from 192.168.1.1: icmp_seq=162 ttl=64 time=0.407 ms
64 bytes from 192.168.1.1: icmp_seq=163 ttl=64 time=0.352 ms
64 bytes from 192.168.1.1: icmp_seq=164 ttl=64 time=0.351 ms
64 bytes from 192.168.1.1: icmp_seq=165 ttl=64 time=0.378 ms
64 bytes from 192.168.1.1: icmp_seq=166 ttl=64 time=0.351 ms

64 bytes from 192.168.1.1: icmp_seq=167 ttl=64 time=0.353 ms
fg
^C
--- 192.168.1.1 ping statistics ---
167 packets transmitted, 158 received, +9 errors, 5.38922% packet loss,
time 791ms
rtt min/avg/max/mdev = 0.330/1511.202/9886.157/2799.059 ms, pipe 10
root@imx8qmmek:~# dmesg -c
[ 1620.382457] configfs-gadget gadget: suspend
[ 1622.840007] configfs-gadget gadget: resume
[ 1631.275452] configfs-gadget gadget: suspend
[ 1633.839442] configfs-gadget gadget: resume
[ 1648.257668] configfs-gadget gadget: suspend
[ 1657.837955] configfs-gadget gadget: resume
[ 1669.252874] configfs-gadget gadget: suspend
[ 1679.836613] configfs-gadget gadget: resume
[ 1692.245250] configfs-gadget gadget: suspend
[ 1695.835642] configfs-gadget gadget: resume
[ 1711.255216] configfs-gadget gadget: suspend

Peter


> From 7cdc1cebe4092393e1de33f59fd2f1cd4355d494 Mon Sep 17 00:00:00 2001
> From: Bowe Neuenschwander <bowe.neuenschwander@garmin.com>
> Date: Tue, 10 Nov 2020 15:55:51 -0600
> Subject: [PATCH 1/2] usb: gadget: eem: Add suspend/resume handling
> 
> Add suspend/resume handling to the USB Gadget EEM driver.  On suspend,
> disconnect the interface; on resume, attempt to reconnect the interface.
> 
> Change-Id: I1c7a2bb1d68a99c774a415b13f6cdabb507ada92
> ---
>  drivers/usb/gadget/function/f_eem.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/f_eem.c b/drivers/usb/gadget/function/f_eem.c
> index cad35a502d3f..4fbdbb8ee295 100644
> --- a/drivers/usb/gadget/function/f_eem.c
> +++ b/drivers/usb/gadget/function/f_eem.c
> @@ -238,6 +238,33 @@ static void eem_disable(struct usb_function *f)
>  		gether_disconnect(&eem->port);
>  }
>  
> +static void eem_suspend(struct usb_function *f)
> +{
> +	struct f_eem		*eem = func_to_eem(f);
> +	struct usb_composite_dev *cdev = f->config->cdev;
> +
> +	DBG(cdev, "eem suspended\n");
> +
> +	if (eem->port.in_ep->enabled)
> +		gether_disconnect(&eem->port);
> +}
> +
> +static void eem_resume(struct usb_function *f)
> +{
> +	struct f_eem		*eem = func_to_eem(f);
> +	struct usb_composite_dev *cdev = f->config->cdev;
> +	struct net_device	*net;
> +
> +	DBG(cdev, "eem resumed\n");
> +
> +	if (!eem->port.in_ep->enabled) {
> +		net = gether_connect(&eem->port);
> +		if (IS_ERR(net))
> +			ERROR(cdev, "%s: eem resume failed, err %d\n",
> +			      f->name, PTR_ERR(net));
> +	}
> +}
> +
>  /*-------------------------------------------------------------------------*/
>  
>  /* EEM function driver setup/binding */
> @@ -636,6 +663,8 @@ static struct usb_function *eem_alloc(struct usb_function_instance *fi)
>  	eem->port.func.set_alt = eem_set_alt;
>  	eem->port.func.setup = eem_setup;
>  	eem->port.func.disable = eem_disable;
> +	eem->port.func.suspend = eem_suspend;
> +	eem->port.func.resume = eem_resume;
>  	eem->port.func.free_func = eem_free;
>  	eem->port.wrap = eem_wrap;
>  	eem->port.unwrap = eem_unwrap;
> -- 
> 2.29.2
> 


-- 

Thanks,
Peter Chen