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 <[email protected]>
---
drivers/usb/gadget/function/u_ether.c | 47 +++++++++++++++++++++++++++--------
1 file changed, 37 insertions(+), 10 deletions(-)
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;
--
2.7.4
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/[email protected]/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 <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests