Re: [PATCH v13 6/6] usb: gadget: f_ecm: Add suspend/resume and remote wakeup support

[Thinh Nguyen]

On Fri, Mar 24, 2023, Elson Roy Serrao wrote:
> When host sends suspend notification to the device, handle
> the suspend callbacks in the function driver. Depending on
> the remote wakeup capability the device can either trigger a
> remote wakeup or wait for the host initiated resume to resume
> data transfer.
> 
> Signed-off-by: Elson Roy Serrao <quic_eserrao@xxxxxxxxxxx>
> ---
>  drivers/usb/gadget/function/f_ecm.c   | 22 ++++++++++++
>  drivers/usb/gadget/function/u_ether.c | 63 +++++++++++++++++++++++++++++++++++
>  drivers/usb/gadget/function/u_ether.h |  4 +++
>  3 files changed, 89 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
> index a7ab30e..c6e63ad 100644
> --- a/drivers/usb/gadget/function/f_ecm.c
> +++ b/drivers/usb/gadget/function/f_ecm.c
> @@ -885,6 +885,26 @@ static struct usb_function_instance *ecm_alloc_inst(void)
>  	return &opts->func_inst;
>  }
>  
> +static void ecm_suspend(struct usb_function *f)
> +{
> +	struct f_ecm *ecm = func_to_ecm(f);
> +	struct usb_composite_dev *cdev = ecm->port.func.config->cdev;
> +
> +	DBG(cdev, "ECM Suspend\n");
> +
> +	gether_suspend(&ecm->port);
> +}
> +
> +static void ecm_resume(struct usb_function *f)
> +{
> +	struct f_ecm *ecm = func_to_ecm(f);
> +	struct usb_composite_dev *cdev = ecm->port.func.config->cdev;
> +
> +	DBG(cdev, "ECM Resume\n");
> +
> +	gether_resume(&ecm->port);
> +}
> +
>  static void ecm_free(struct usb_function *f)
>  {
>  	struct f_ecm *ecm;
> @@ -952,6 +972,8 @@ static struct usb_function *ecm_alloc(struct usb_function_instance *fi)
>  	ecm->port.func.setup = ecm_setup;
>  	ecm->port.func.disable = ecm_disable;
>  	ecm->port.func.free_func = ecm_free;
> +	ecm->port.func.suspend = ecm_suspend;
> +	ecm->port.func.resume = ecm_resume;
>  
>  	return &ecm->port.func;
>  }
> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
> index f259975..6956ad8 100644
> --- a/drivers/usb/gadget/function/u_ether.c
> +++ b/drivers/usb/gadget/function/u_ether.c
> @@ -437,6 +437,20 @@ static inline int is_promisc(u16 cdc_filter)
>  	return cdc_filter & USB_CDC_PACKET_TYPE_PROMISCUOUS;
>  }
>  
> +static int ether_wakeup_host(struct gether *port)
> +{
> +	int			ret;
> +	struct usb_function	*func = &port->func;
> +	struct usb_gadget	*gadget = func->config->cdev->gadget;
> +
> +	if (func->func_suspended)
> +		ret = usb_func_wakeup(func);
> +	else
> +		ret = usb_gadget_wakeup(gadget);
> +
> +	return ret;
> +}
> +
>  static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
>  					struct net_device *net)
>  {
> @@ -456,6 +470,15 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
>  		in = NULL;
>  		cdc_filter = 0;
>  	}
> +
> +	if (dev->port_usb && dev->port_usb->is_suspend) {
> +		DBG(dev, "Port suspended. Triggering wakeup\n");
> +		netif_stop_queue(net);
> +		spin_unlock_irqrestore(&dev->lock, flags);
> +		ether_wakeup_host(dev->port_usb);
> +		return NETDEV_TX_BUSY;
> +	}
> +
>  	spin_unlock_irqrestore(&dev->lock, flags);
>  
>  	if (!in) {
> @@ -1014,6 +1037,45 @@ int gether_set_ifname(struct net_device *net, const char *name, int len)
>  }
>  EXPORT_SYMBOL_GPL(gether_set_ifname);
>  
> +void gether_suspend(struct gether *link)
> +{
> +	struct eth_dev *dev = link->ioport;
> +	unsigned long flags;
> +
> +	if (!dev)
> +		return;
> +
> +	if (atomic_read(&dev->tx_qlen)) {
> +		/*
> +		 * There is a transfer in progress. So we trigger a remote
> +		 * wakeup to inform the host.
> +		 */
> +		ether_wakeup_host(dev->port_usb);
> +		return;
> +	}
> +	spin_lock_irqsave(&dev->lock, flags);
> +	link->is_suspend = true;
> +	spin_unlock_irqrestore(&dev->lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(gether_suspend);
> +
> +void gether_resume(struct gether *link)
> +{
> +	struct eth_dev *dev = link->ioport;
> +	unsigned long flags;
> +
> +	if (!dev)
> +		return;
> +
> +	if (netif_queue_stopped(dev->net))
> +		netif_start_queue(dev->net);
> +
> +	spin_lock_irqsave(&dev->lock, flags);
> +	link->is_suspend = false;
> +	spin_unlock_irqrestore(&dev->lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(gether_resume);
> +
>  /*
>   * gether_cleanup - remove Ethernet-over-USB device
>   * Context: may sleep
> @@ -1176,6 +1238,7 @@ void gether_disconnect(struct gether *link)
>  
>  	spin_lock(&dev->lock);
>  	dev->port_usb = NULL;
> +	link->is_suspend = false;
>  	spin_unlock(&dev->lock);
>  }
>  EXPORT_SYMBOL_GPL(gether_disconnect);
> diff --git a/drivers/usb/gadget/function/u_ether.h b/drivers/usb/gadget/function/u_ether.h
> index 4014454..851ee10 100644
> --- a/drivers/usb/gadget/function/u_ether.h
> +++ b/drivers/usb/gadget/function/u_ether.h
> @@ -79,6 +79,7 @@ struct gether {
>  	/* called on network open/close */
>  	void				(*open)(struct gether *);
>  	void				(*close)(struct gether *);
> +	bool				is_suspend;
>  };
>  
>  #define	DEFAULT_FILTER	(USB_CDC_PACKET_TYPE_BROADCAST \
> @@ -258,6 +259,9 @@ int gether_set_ifname(struct net_device *net, const char *name, int len);
>  
>  void gether_cleanup(struct eth_dev *dev);
>  
> +void gether_suspend(struct gether *link);
> +void gether_resume(struct gether *link);
> +
>  /* connect/disconnect is handled by individual functions */
>  struct net_device *gether_connect(struct gether *);
>  void gether_disconnect(struct gether *);
> -- 
> 2.7.4
> 

For the remote wakeup part of this patch:

Reviewed-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>

Thanks,
Thinh