diff mbox series

[1/2] usb: function: u_ether: Handle rx requests during suspend/resume

Message ID 1683827311-1462-2-git-send-email-quic_eserrao@quicinc.com (mailing list archive)
State New, archived
Headers show
Series Allow dwc3 runtime suspend during bus suspend event | expand

Commit Message

Elson Roy Serrao May 11, 2023, 5:48 p.m. UTC 
Some UDCs might have a vote against runtime suspend if there is any
request queued by the function driver. This would block the UDC driver
to enter runtime suspend state when the host sends bus suspend
notification. While tx requests get dequeued after completion, rx
requests always remain queued for the next OUT data to be handled. Since
during bus suspend scenario there are no active OUT transfers we can
dequeue these requests when the function driver suspend callback gets
called and queue them back during resume callback. Implement this
mechanism by adding a new list for queued requests.

Also move the gether_wakeup_host API to work queue context to better
align with the remote wakeup op's synchronous operation.

Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
---
 drivers/usb/gadget/function/u_ether.c | 47 +++++++++++++++++++++++++++--------
 1 file changed, 37 insertions(+), 10 deletions(-)

Comments

kernel test robot May 11, 2023, 7:33 p.m. UTC | #1 
Hi Elson,

kernel test robot noticed the following build warnings:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus linus/master v6.4-rc1 next-20230511]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Elson-Roy-Serrao/usb-function-u_ether-Handle-rx-requests-during-suspend-resume/20230512-015036
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/1683827311-1462-2-git-send-email-quic_eserrao%40quicinc.com
patch subject: [PATCH 1/2] usb: function: u_ether: Handle rx requests during suspend/resume
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230512/202305120311.KEGxEo2Z-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/90c8743982bad3c71cd13e366efa6b596dd24120
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Elson-Roy-Serrao/usb-function-u_ether-Handle-rx-requests-during-suspend-resume/20230512-015036
        git checkout 90c8743982bad3c71cd13e366efa6b596dd24120
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/usb/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305120311.KEGxEo2Z-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/usb/gadget/function/u_ether.c: In function 'ether_wakeup_work':
>> drivers/usb/gadget/function/u_ether.c:442:33: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
     442 |         int                     ret;
         |                                 ^~~
   drivers/usb/gadget/function/u_ether.c: In function 'gether_suspend':
>> drivers/usb/gadget/function/u_ether.c:1049:13: warning: variable 'status' set but not used [-Wunused-but-set-variable]
    1049 |         int status;
         |             ^~~~~~


vim +/ret +442 drivers/usb/gadget/function/u_ether.c

2b3d942c487808 drivers/usb/gadget/u_ether.c          David Brownell   2008-06-19  439  
90c8743982bad3 drivers/usb/gadget/function/u_ether.c Elson Roy Serrao 2023-05-11  440  static void ether_wakeup_work(struct work_struct *w)
0a1af6dfa0772f drivers/usb/gadget/function/u_ether.c Elson Roy Serrao 2023-03-24  441  {
0a1af6dfa0772f drivers/usb/gadget/function/u_ether.c Elson Roy Serrao 2023-03-24 @442  	int			ret;
90c8743982bad3 drivers/usb/gadget/function/u_ether.c Elson Roy Serrao 2023-05-11  443  	struct eth_dev		*dev = container_of(w, struct eth_dev, wakeup_work);
90c8743982bad3 drivers/usb/gadget/function/u_ether.c Elson Roy Serrao 2023-05-11  444  	struct gether		*port = dev->port_usb;
0a1af6dfa0772f drivers/usb/gadget/function/u_ether.c Elson Roy Serrao 2023-03-24  445  	struct usb_function	*func = &port->func;
0a1af6dfa0772f drivers/usb/gadget/function/u_ether.c Elson Roy Serrao 2023-03-24  446  	struct usb_gadget	*gadget = func->config->cdev->gadget;
0a1af6dfa0772f drivers/usb/gadget/function/u_ether.c Elson Roy Serrao 2023-03-24  447  
0a1af6dfa0772f drivers/usb/gadget/function/u_ether.c Elson Roy Serrao 2023-03-24  448  	if (func->func_suspended)
0a1af6dfa0772f drivers/usb/gadget/function/u_ether.c Elson Roy Serrao 2023-03-24  449  		ret = usb_func_wakeup(func);
0a1af6dfa0772f drivers/usb/gadget/function/u_ether.c Elson Roy Serrao 2023-03-24  450  	else
0a1af6dfa0772f drivers/usb/gadget/function/u_ether.c Elson Roy Serrao 2023-03-24  451  		ret = usb_gadget_wakeup(gadget);
0a1af6dfa0772f drivers/usb/gadget/function/u_ether.c Elson Roy Serrao 2023-03-24  452  }
0a1af6dfa0772f drivers/usb/gadget/function/u_ether.c Elson Roy Serrao 2023-03-24  453
diff mbox series

Patch

diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
index 6956ad8..ce0a6a7 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -61,7 +61,7 @@  struct eth_dev {
 	struct usb_gadget	*gadget;
 
 	spinlock_t		req_lock;	/* guard {rx,tx}_reqs */
-	struct list_head	tx_reqs, rx_reqs;
+	struct list_head	tx_reqs, rx_reqs, rx_queued_reqs;
 	atomic_t		tx_qlen;
 
 	struct sk_buff_head	rx_frames;
@@ -74,7 +74,7 @@  struct eth_dev {
 						struct sk_buff *skb,
 						struct sk_buff_head *list);
 
-	struct work_struct	work;
+	struct work_struct	work, wakeup_work;
 
 	unsigned long		todo;
 #define	WORK_RX_MEMORY		0
@@ -212,7 +212,7 @@  rx_submit(struct eth_dev *dev, struct usb_request *req, gfp_t gfp_flags)
 		if (skb)
 			dev_kfree_skb_any(skb);
 		spin_lock_irqsave(&dev->req_lock, flags);
-		list_add(&req->list, &dev->rx_reqs);
+		list_move_tail(&req->list, &dev->rx_reqs);
 		spin_unlock_irqrestore(&dev->req_lock, flags);
 	}
 	return retval;
@@ -302,7 +302,7 @@  static void rx_complete(struct usb_ep *ep, struct usb_request *req)
 	if (!netif_running(dev->net)) {
 clean:
 		spin_lock(&dev->req_lock);
-		list_add(&req->list, &dev->rx_reqs);
+		list_move_tail(&req->list, &dev->rx_reqs);
 		spin_unlock(&dev->req_lock);
 		req = NULL;
 	}
@@ -377,7 +377,7 @@  static void rx_fill(struct eth_dev *dev, gfp_t gfp_flags)
 	spin_lock_irqsave(&dev->req_lock, flags);
 	while (!list_empty(&dev->rx_reqs)) {
 		req = list_first_entry(&dev->rx_reqs, struct usb_request, list);
-		list_del_init(&req->list);
+		list_move_tail(&req->list, &dev->rx_queued_reqs);
 		spin_unlock_irqrestore(&dev->req_lock, flags);
 
 		if (rx_submit(dev, req, gfp_flags) < 0) {
@@ -437,9 +437,11 @@  static inline int is_promisc(u16 cdc_filter)
 	return cdc_filter & USB_CDC_PACKET_TYPE_PROMISCUOUS;
 }
 
-static int ether_wakeup_host(struct gether *port)
+static void ether_wakeup_work(struct work_struct *w)
 {
 	int			ret;
+	struct eth_dev		*dev = container_of(w, struct eth_dev, wakeup_work);
+	struct gether		*port = dev->port_usb;
 	struct usb_function	*func = &port->func;
 	struct usb_gadget	*gadget = func->config->cdev->gadget;
 
@@ -447,8 +449,6 @@  static int ether_wakeup_host(struct gether *port)
 		ret = usb_func_wakeup(func);
 	else
 		ret = usb_gadget_wakeup(gadget);
-
-	return ret;
 }
 
 static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
@@ -475,7 +475,7 @@  static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
 		DBG(dev, "Port suspended. Triggering wakeup\n");
 		netif_stop_queue(net);
 		spin_unlock_irqrestore(&dev->lock, flags);
-		ether_wakeup_host(dev->port_usb);
+		schedule_work(&dev->wakeup_work);
 		return NETDEV_TX_BUSY;
 	}
 
@@ -753,8 +753,10 @@  struct eth_dev *gether_setup_name(struct usb_gadget *g,
 	spin_lock_init(&dev->lock);
 	spin_lock_init(&dev->req_lock);
 	INIT_WORK(&dev->work, eth_work);
+	INIT_WORK(&dev->wakeup_work, ether_wakeup_work);
 	INIT_LIST_HEAD(&dev->tx_reqs);
 	INIT_LIST_HEAD(&dev->rx_reqs);
+	INIT_LIST_HEAD(&dev->rx_queued_reqs);
 
 	skb_queue_head_init(&dev->rx_frames);
 
@@ -824,8 +826,10 @@  struct net_device *gether_setup_name_default(const char *netname)
 	spin_lock_init(&dev->lock);
 	spin_lock_init(&dev->req_lock);
 	INIT_WORK(&dev->work, eth_work);
+	INIT_WORK(&dev->wakeup_work, ether_wakeup_work);
 	INIT_LIST_HEAD(&dev->tx_reqs);
 	INIT_LIST_HEAD(&dev->rx_reqs);
+	INIT_LIST_HEAD(&dev->rx_queued_reqs);
 
 	skb_queue_head_init(&dev->rx_frames);
 
@@ -1040,7 +1044,9 @@  EXPORT_SYMBOL_GPL(gether_set_ifname);
 void gether_suspend(struct gether *link)
 {
 	struct eth_dev *dev = link->ioport;
+	struct usb_request *req;
 	unsigned long flags;
+	int status;
 
 	if (!dev)
 		return;
@@ -1050,9 +1056,20 @@  void gether_suspend(struct gether *link)
 		 * There is a transfer in progress. So we trigger a remote
 		 * wakeup to inform the host.
 		 */
-		ether_wakeup_host(dev->port_usb);
+		schedule_work(&dev->wakeup_work);
 		return;
 	}
+	/* Dequeue the submitted requests. */
+	spin_lock(&dev->req_lock);
+	while (!list_empty(&dev->rx_queued_reqs)) {
+		req = list_last_entry(&dev->rx_queued_reqs, struct usb_request, list);
+		list_move_tail(&req->list, &dev->rx_reqs);
+		spin_unlock(&dev->req_lock);
+		status = usb_ep_dequeue(dev->port_usb->out_ep, req);
+		spin_lock(&dev->req_lock);
+	}
+	spin_unlock(&dev->req_lock);
+
 	spin_lock_irqsave(&dev->lock, flags);
 	link->is_suspend = true;
 	spin_unlock_irqrestore(&dev->lock, flags);
@@ -1067,6 +1084,7 @@  void gether_resume(struct gether *link)
 	if (!dev)
 		return;
 
+	defer_kevent(dev, WORK_RX_MEMORY);
 	if (netif_queue_stopped(dev->net))
 		netif_start_queue(dev->net);
 
@@ -1228,6 +1246,15 @@  void gether_disconnect(struct gether *link)
 		usb_ep_free_request(link->out_ep, req);
 		spin_lock(&dev->req_lock);
 	}
+
+	while (!list_empty(&dev->rx_queued_reqs)) {
+		req = list_first_entry(&dev->rx_queued_reqs, struct usb_request, list);
+		list_del(&req->list);
+
+		spin_unlock(&dev->req_lock);
+		usb_ep_free_request(link->out_ep, req);
+		spin_lock(&dev->req_lock);
+	}
 	spin_unlock(&dev->req_lock);
 	link->out_ep->desc = NULL;