* [PATCH 0/2] Allow dwc3 runtime suspend during bus suspend event @ 2023-05-11 17:48 Elson Roy Serrao 2023-05-11 17:48 ` [PATCH 1/2] usb: function: u_ether: Handle rx requests during suspend/resume Elson Roy Serrao 2023-05-11 17:48 ` [PATCH 2/2] usb: dwc3: Modify runtime pm ops to handle bus suspend Elson Roy Serrao 0 siblings, 2 replies; 6+ messages in thread From: Elson Roy Serrao @ 2023-05-11 17:48 UTC (permalink / raw) To: gregkh, Thinh.Nguyen Cc: linux-kernel, linux-usb, quic_wcheng, quic_jackp, Elson Roy Serrao When a USB link is idle, the host sends a bus suspend event to the device so that the device can save power. But true power savings during bus suspend can be seen only if we let the USB controller enter low power mode and turn off the clocks. Vendor drivers may have their own runtime power management framework to power up/down the controller. But since vendor drivers' runtime suspend/resume routines depend on the dwc3 child node we would need a framework to trigger dwc3 runtime pm ops whenever a bus suspend is received. If the device wants to exit from bus suspend state it can send a wakeup signal to the host by first bringing out the controller from low power mode. This series implements the needed framework to achieve this functionality when a bus suspend interupt is received. The series is organized in below fashion: Patch 1: This includes the modification needed from function drivers to let UDC enter low power mode. Patch 2: This has the modification needed in the UDC driver to trigger runtime suspend whene a bus suspend interrupt is received. This also handles resume and remote wakeup features from power management perspective. Elson Roy Serrao (2): usb: function: u_ether: Handle rx requests during suspend/resume usb: dwc3: Modify runtime pm ops to handle bus suspend drivers/usb/dwc3/core.c | 19 ++++++++++++++ drivers/usb/dwc3/gadget.c | 40 +++++++++++++++++++++++------ drivers/usb/gadget/function/u_ether.c | 47 +++++++++++++++++++++++++++-------- 3 files changed, 89 insertions(+), 17 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] usb: function: u_ether: Handle rx requests during suspend/resume 2023-05-11 17:48 [PATCH 0/2] Allow dwc3 runtime suspend during bus suspend event Elson Roy Serrao @ 2023-05-11 17:48 ` Elson Roy Serrao 2023-05-11 19:33 ` kernel test robot 2023-05-11 17:48 ` [PATCH 2/2] usb: dwc3: Modify runtime pm ops to handle bus suspend Elson Roy Serrao 1 sibling, 1 reply; 6+ messages in thread From: Elson Roy Serrao @ 2023-05-11 17:48 UTC (permalink / raw) To: gregkh, Thinh.Nguyen Cc: linux-kernel, linux-usb, quic_wcheng, quic_jackp, Elson Roy Serrao 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(-) 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] usb: function: u_ether: Handle rx requests during suspend/resume 2023-05-11 17:48 ` [PATCH 1/2] usb: function: u_ether: Handle rx requests during suspend/resume Elson Roy Serrao @ 2023-05-11 19:33 ` kernel test robot 0 siblings, 0 replies; 6+ messages in thread From: kernel test robot @ 2023-05-11 19:33 UTC (permalink / raw) To: Elson Roy Serrao, gregkh, Thinh.Nguyen Cc: oe-kbuild-all, linux-kernel, linux-usb, quic_wcheng, quic_jackp, Elson Roy Serrao 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 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] usb: dwc3: Modify runtime pm ops to handle bus suspend 2023-05-11 17:48 [PATCH 0/2] Allow dwc3 runtime suspend during bus suspend event Elson Roy Serrao 2023-05-11 17:48 ` [PATCH 1/2] usb: function: u_ether: Handle rx requests during suspend/resume Elson Roy Serrao @ 2023-05-11 17:48 ` Elson Roy Serrao 2023-05-12 4:18 ` Dan Carpenter 1 sibling, 1 reply; 6+ messages in thread From: Elson Roy Serrao @ 2023-05-11 17:48 UTC (permalink / raw) To: gregkh, Thinh.Nguyen Cc: linux-kernel, linux-usb, quic_wcheng, quic_jackp, Elson Roy Serrao For bus suspend scenario we should skip controller halt as we still have the usb cable connected. So modify the runtime pm ops to handle bus suspend scenario. Also invoke autosuspend when device receives U3 notification so that controller can enter runtime suspended state. Ensure that the controller is brought out of runtime suspend before triggering remote wakeup. Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com> --- drivers/usb/dwc3/core.c | 19 +++++++++++++++++++ drivers/usb/dwc3/gadget.c | 40 +++++++++++++++++++++++++++++++++------- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 0beaab9..b00d1e8 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -2108,6 +2108,12 @@ static int dwc3_runtime_suspend(struct device *dev) { struct dwc3 *dwc = dev_get_drvdata(dev); int ret; + u32 reg; + + reg = dwc3_readl(dwc->regs, DWC3_DSTS); + /* For bus suspend case do not halt the controller */ + if (DWC3_DSTS_USBLNKST(reg) == DWC3_LINK_STATE_U3 && dwc->connected) + return 0; if (dwc3_runtime_checks(dwc)) return -EBUSY; @@ -2124,6 +2130,13 @@ static int dwc3_runtime_resume(struct device *dev) struct dwc3 *dwc = dev_get_drvdata(dev); int ret; + /* resume from bus suspend */ + if (dwc->connected) { + dwc3_gadget_process_pending_events(dwc); + pm_runtime_mark_last_busy(dev); + return 0; + } + ret = dwc3_resume_common(dwc, PMSG_AUTO_RESUME); if (ret) return ret; @@ -2146,9 +2159,14 @@ static int dwc3_runtime_resume(struct device *dev) static int dwc3_runtime_idle(struct device *dev) { struct dwc3 *dwc = dev_get_drvdata(dev); + u32 reg; switch (dwc->current_dr_role) { case DWC3_GCTL_PRTCAP_DEVICE: + reg = dwc3_readl(dwc->regs, DWC3_DSTS); + /* for bus suspend case return success */ + if (DWC3_DSTS_USBLNKST(reg) == DWC3_LINK_STATE_U3 && dwc->connected) + goto autosuspend; if (dwc3_runtime_checks(dwc)) return -EBUSY; break; @@ -2158,6 +2176,7 @@ static int dwc3_runtime_idle(struct device *dev) break; } +autosuspend: pm_runtime_mark_last_busy(dev); pm_runtime_autosuspend(dev); diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index c0ca4d1..d4da83b 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2400,15 +2400,21 @@ static int dwc3_gadget_wakeup(struct usb_gadget *g) return -EINVAL; } - spin_lock_irqsave(&dwc->lock, flags); if (!dwc->gadget->wakeup_armed) { dev_err(dwc->dev, "not armed for remote wakeup\n"); - spin_unlock_irqrestore(&dwc->lock, flags); return -EINVAL; } - ret = __dwc3_gadget_wakeup(dwc, true); + ret = pm_runtime_get_sync(dwc->dev); + if (ret) { + pm_runtime_put(dwc->dev); + return ret; + } + + spin_lock_irqsave(&dwc->lock, flags); + ret = __dwc3_gadget_wakeup(dwc, true); spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_noidle(dwc->dev); return ret; } @@ -2427,6 +2433,12 @@ static int dwc3_gadget_func_wakeup(struct usb_gadget *g, int intf_id) return -EINVAL; } + ret = pm_runtime_get_sync(dwc->dev); + if (ret) { + pm_runtime_put(dwc->dev); + return ret; + } + spin_lock_irqsave(&dwc->lock, flags); /* * If the link is in U3, signal for remote wakeup and wait for the @@ -2437,6 +2449,7 @@ static int dwc3_gadget_func_wakeup(struct usb_gadget *g, int intf_id) ret = __dwc3_gadget_wakeup(dwc, false); if (ret) { spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_noidle(dwc->dev); return -EINVAL; } dwc3_resume_gadget(dwc); @@ -2450,6 +2463,7 @@ static int dwc3_gadget_func_wakeup(struct usb_gadget *g, int intf_id) dev_err(dwc->dev, "function remote wakeup failed, ret:%d\n", ret); spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_noidle(dwc->dev); return ret; } @@ -2711,21 +2725,23 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) /* * Avoid issuing a runtime resume if the device is already in the * suspended state during gadget disconnect. DWC3 gadget was already - * halted/stopped during runtime suspend. + * halted/stopped during runtime suspend except for bus suspend case + * where we would have skipped the controller halt. */ if (!is_on) { pm_runtime_barrier(dwc->dev); - if (pm_runtime_suspended(dwc->dev)) + if (pm_runtime_suspended(dwc->dev) && !dwc->connected) return 0; } /* * Check the return value for successful resume, or error. For a * successful resume, the DWC3 runtime PM resume routine will handle - * the run stop sequence, so avoid duplicate operations here. + * the run stop sequence except for bus resume case, so avoid + * duplicate operations here. */ ret = pm_runtime_get_sync(dwc->dev); - if (!ret || ret < 0) { + if ((!ret && !dwc->connected) || ret < 0) { pm_runtime_put(dwc->dev); return 0; } @@ -4307,6 +4323,8 @@ static void dwc3_gadget_suspend_interrupt(struct dwc3 *dwc, dwc3_suspend_gadget(dwc); dwc->link_state = next; + pm_runtime_mark_last_busy(dwc->dev); + pm_request_autosuspend(dwc->dev); } static void dwc3_gadget_interrupt(struct dwc3 *dwc, @@ -4697,7 +4715,15 @@ void dwc3_gadget_process_pending_events(struct dwc3 *dwc) { if (dwc->pending_events) { dwc3_interrupt(dwc->irq_gadget, dwc->ev_buf); + pm_runtime_put(dwc->dev); dwc->pending_events = false; enable_irq(dwc->irq_gadget); + /* + * We have only stored the pending events as part + * of dwc3_interrupt() above, but those events are + * not yet handled. So explicitly invoke the + * interrupt handler for handling those events. + */ + dwc3_thread_interrupt(dwc->irq_gadget, dwc->ev_buf); } } -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] usb: dwc3: Modify runtime pm ops to handle bus suspend 2023-05-11 17:48 ` [PATCH 2/2] usb: dwc3: Modify runtime pm ops to handle bus suspend Elson Roy Serrao @ 2023-05-12 4:18 ` Dan Carpenter 2023-05-12 17:18 ` Elson Serrao 0 siblings, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2023-05-12 4:18 UTC (permalink / raw) To: oe-kbuild, Elson Roy Serrao, gregkh, Thinh.Nguyen Cc: lkp, oe-kbuild-all, linux-kernel, linux-usb, quic_wcheng, quic_jackp, Elson Roy Serrao Hi Elson, kernel test robot noticed the following build warnings: 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-3-git-send-email-quic_eserrao%40quicinc.com patch subject: [PATCH 2/2] usb: dwc3: Modify runtime pm ops to handle bus suspend config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20230512/202305120709.tCFYCtsd-lkp@intel.com/config) compiler: gcc-11 (Debian 11.3.0-12) 11.3.0 If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Reported-by: Dan Carpenter <error27@gmail.com> | Link: https://lore.kernel.org/r/202305120709.tCFYCtsd-lkp@intel.com/ New smatch warnings: drivers/usb/dwc3/gadget.c:2409 dwc3_gadget_wakeup() warn: pm_runtime_get_sync() also returns 1 on success drivers/usb/dwc3/gadget.c:2437 dwc3_gadget_func_wakeup() warn: pm_runtime_get_sync() also returns 1 on success Old smatch warnings: drivers/usb/dwc3/gadget.c:1648 __dwc3_gadget_kick_transfer() warn: missing error code? 'ret' drivers/usb/dwc3/gadget.c:2744 dwc3_gadget_pullup() warn: pm_runtime_get_sync() also returns 1 on success vim +2409 drivers/usb/dwc3/gadget.c 218ef7b647e336 Felipe Balbi 2016-04-04 2392 static int dwc3_gadget_wakeup(struct usb_gadget *g) 218ef7b647e336 Felipe Balbi 2016-04-04 2393 { 218ef7b647e336 Felipe Balbi 2016-04-04 2394 struct dwc3 *dwc = gadget_to_dwc(g); 218ef7b647e336 Felipe Balbi 2016-04-04 2395 unsigned long flags; 218ef7b647e336 Felipe Balbi 2016-04-04 2396 int ret; 218ef7b647e336 Felipe Balbi 2016-04-04 2397 047161686b813a Elson Roy Serrao 2023-03-24 2398 if (!dwc->wakeup_configured) { 047161686b813a Elson Roy Serrao 2023-03-24 2399 dev_err(dwc->dev, "remote wakeup not configured\n"); 047161686b813a Elson Roy Serrao 2023-03-24 2400 return -EINVAL; 047161686b813a Elson Roy Serrao 2023-03-24 2401 } 047161686b813a Elson Roy Serrao 2023-03-24 2402 047161686b813a Elson Roy Serrao 2023-03-24 2403 if (!dwc->gadget->wakeup_armed) { 047161686b813a Elson Roy Serrao 2023-03-24 2404 dev_err(dwc->dev, "not armed for remote wakeup\n"); 047161686b813a Elson Roy Serrao 2023-03-24 2405 return -EINVAL; 047161686b813a Elson Roy Serrao 2023-03-24 2406 } 047161686b813a Elson Roy Serrao 2023-03-24 2407 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2408 ret = pm_runtime_get_sync(dwc->dev); 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 @2409 if (ret) { The checker is correct. These days it's better to use the pm_runtime_resume_and_get() function instead of pm_runtime_get_sync(). 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2410 pm_runtime_put(dwc->dev); 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2411 return ret; 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2412 } 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2413 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2414 spin_lock_irqsave(&dwc->lock, flags); 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2415 ret = __dwc3_gadget_wakeup(dwc, true); 72246da40f3719 Felipe Balbi 2011-08-19 2416 spin_unlock_irqrestore(&dwc->lock, flags); 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2417 pm_runtime_put_noidle(dwc->dev); 72246da40f3719 Felipe Balbi 2011-08-19 2418 72246da40f3719 Felipe Balbi 2011-08-19 2419 return ret; 72246da40f3719 Felipe Balbi 2011-08-19 2420 } 72246da40f3719 Felipe Balbi 2011-08-19 2421 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2422 static void dwc3_resume_gadget(struct dwc3 *dwc); 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2423 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2424 static int dwc3_gadget_func_wakeup(struct usb_gadget *g, int intf_id) 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2425 { 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2426 struct dwc3 *dwc = gadget_to_dwc(g); 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2427 unsigned long flags; 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2428 int ret; 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2429 int link_state; 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2430 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2431 if (!dwc->wakeup_configured) { 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2432 dev_err(dwc->dev, "remote wakeup not configured\n"); 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2433 return -EINVAL; 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2434 } 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2435 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2436 ret = pm_runtime_get_sync(dwc->dev); 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 @2437 if (ret) { Same. 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2438 pm_runtime_put(dwc->dev); 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2439 return ret; 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2440 } 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2441 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2442 spin_lock_irqsave(&dwc->lock, flags); 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2443 /* 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2444 * If the link is in U3, signal for remote wakeup and wait for the 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2445 * link to transition to U0 before sending device notification. 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2446 */ 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2447 link_state = dwc3_gadget_get_link_state(dwc); 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2448 if (link_state == DWC3_LINK_STATE_U3) { 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2449 ret = __dwc3_gadget_wakeup(dwc, false); 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2450 if (ret) { 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2451 spin_unlock_irqrestore(&dwc->lock, flags); 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2452 pm_runtime_put_noidle(dwc->dev); 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2453 return -EINVAL; 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2454 } 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2455 dwc3_resume_gadget(dwc); 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2456 dwc->link_state = DWC3_LINK_STATE_U0; 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2457 } 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2458 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2459 ret = dwc3_send_gadget_generic_command(dwc, DWC3_DGCMD_DEV_NOTIFICATION, 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2460 DWC3_DGCMDPAR_DN_FUNC_WAKE | 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2461 DWC3_DGCMDPAR_INTF_SEL(intf_id)); 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2462 if (ret) 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2463 dev_err(dwc->dev, "function remote wakeup failed, ret:%d\n", ret); 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2464 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2465 spin_unlock_irqrestore(&dwc->lock, flags); 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2466 pm_runtime_put_noidle(dwc->dev); 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2467 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2468 return ret; 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2469 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] usb: dwc3: Modify runtime pm ops to handle bus suspend 2023-05-12 4:18 ` Dan Carpenter @ 2023-05-12 17:18 ` Elson Serrao 0 siblings, 0 replies; 6+ messages in thread From: Elson Serrao @ 2023-05-12 17:18 UTC (permalink / raw) To: Dan Carpenter, oe-kbuild, gregkh, Thinh.Nguyen Cc: lkp, oe-kbuild-all, linux-kernel, linux-usb, quic_wcheng, quic_jackp On 5/11/2023 9:18 PM, Dan Carpenter wrote: > Hi Elson, > > kernel test robot noticed the following build warnings: > > 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-3-git-send-email-quic_eserrao%40quicinc.com > patch subject: [PATCH 2/2] usb: dwc3: Modify runtime pm ops to handle bus suspend > config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20230512/202305120709.tCFYCtsd-lkp@intel.com/config) > compiler: gcc-11 (Debian 11.3.0-12) 11.3.0 > > If you fix the issue, kindly add following tag where applicable > | Reported-by: kernel test robot <lkp@intel.com> > | Reported-by: Dan Carpenter <error27@gmail.com> > | Link: https://lore.kernel.org/r/202305120709.tCFYCtsd-lkp@intel.com/ > > New smatch warnings: > drivers/usb/dwc3/gadget.c:2409 dwc3_gadget_wakeup() warn: pm_runtime_get_sync() also returns 1 on success > drivers/usb/dwc3/gadget.c:2437 dwc3_gadget_func_wakeup() warn: pm_runtime_get_sync() also returns 1 on success > > Old smatch warnings: > drivers/usb/dwc3/gadget.c:1648 __dwc3_gadget_kick_transfer() warn: missing error code? 'ret' > drivers/usb/dwc3/gadget.c:2744 dwc3_gadget_pullup() warn: pm_runtime_get_sync() also returns 1 on success > > vim +2409 drivers/usb/dwc3/gadget.c > > 218ef7b647e336 Felipe Balbi 2016-04-04 2392 static int dwc3_gadget_wakeup(struct usb_gadget *g) > 218ef7b647e336 Felipe Balbi 2016-04-04 2393 { > 218ef7b647e336 Felipe Balbi 2016-04-04 2394 struct dwc3 *dwc = gadget_to_dwc(g); > 218ef7b647e336 Felipe Balbi 2016-04-04 2395 unsigned long flags; > 218ef7b647e336 Felipe Balbi 2016-04-04 2396 int ret; > 218ef7b647e336 Felipe Balbi 2016-04-04 2397 > 047161686b813a Elson Roy Serrao 2023-03-24 2398 if (!dwc->wakeup_configured) { > 047161686b813a Elson Roy Serrao 2023-03-24 2399 dev_err(dwc->dev, "remote wakeup not configured\n"); > 047161686b813a Elson Roy Serrao 2023-03-24 2400 return -EINVAL; > 047161686b813a Elson Roy Serrao 2023-03-24 2401 } > 047161686b813a Elson Roy Serrao 2023-03-24 2402 > 047161686b813a Elson Roy Serrao 2023-03-24 2403 if (!dwc->gadget->wakeup_armed) { > 047161686b813a Elson Roy Serrao 2023-03-24 2404 dev_err(dwc->dev, "not armed for remote wakeup\n"); > 047161686b813a Elson Roy Serrao 2023-03-24 2405 return -EINVAL; > 047161686b813a Elson Roy Serrao 2023-03-24 2406 } > 047161686b813a Elson Roy Serrao 2023-03-24 2407 > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2408 ret = pm_runtime_get_sync(dwc->dev); > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 @2409 if (ret) { > > The checker is correct. These days it's better to use the > pm_runtime_resume_and_get() function instead of pm_runtime_get_sync(). > Sure.Thanks! > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2410 pm_runtime_put(dwc->dev); > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2411 return ret; > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2412 } > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2413 > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2414 spin_lock_irqsave(&dwc->lock, flags); > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2415 ret = __dwc3_gadget_wakeup(dwc, true); > 72246da40f3719 Felipe Balbi 2011-08-19 2416 spin_unlock_irqrestore(&dwc->lock, flags); > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2417 pm_runtime_put_noidle(dwc->dev); > 72246da40f3719 Felipe Balbi 2011-08-19 2418 > 72246da40f3719 Felipe Balbi 2011-08-19 2419 return ret; > 72246da40f3719 Felipe Balbi 2011-08-19 2420 } > 72246da40f3719 Felipe Balbi 2011-08-19 2421 > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2422 static void dwc3_resume_gadget(struct dwc3 *dwc); > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2423 > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2424 static int dwc3_gadget_func_wakeup(struct usb_gadget *g, int intf_id) > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2425 { > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2426 struct dwc3 *dwc = gadget_to_dwc(g); > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2427 unsigned long flags; > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2428 int ret; > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2429 int link_state; > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2430 > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2431 if (!dwc->wakeup_configured) { > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2432 dev_err(dwc->dev, "remote wakeup not configured\n"); > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2433 return -EINVAL; > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2434 } > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2435 > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2436 ret = pm_runtime_get_sync(dwc->dev); > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 @2437 if (ret) { > > Same. > > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2438 pm_runtime_put(dwc->dev); > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2439 return ret; > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2440 } > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2441 > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2442 spin_lock_irqsave(&dwc->lock, flags); > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2443 /* > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2444 * If the link is in U3, signal for remote wakeup and wait for the > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2445 * link to transition to U0 before sending device notification. > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2446 */ > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2447 link_state = dwc3_gadget_get_link_state(dwc); > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2448 if (link_state == DWC3_LINK_STATE_U3) { > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2449 ret = __dwc3_gadget_wakeup(dwc, false); > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2450 if (ret) { > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2451 spin_unlock_irqrestore(&dwc->lock, flags); > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2452 pm_runtime_put_noidle(dwc->dev); > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2453 return -EINVAL; > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2454 } > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2455 dwc3_resume_gadget(dwc); > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2456 dwc->link_state = DWC3_LINK_STATE_U0; > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2457 } > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2458 > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2459 ret = dwc3_send_gadget_generic_command(dwc, DWC3_DGCMD_DEV_NOTIFICATION, > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2460 DWC3_DGCMDPAR_DN_FUNC_WAKE | > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2461 DWC3_DGCMDPAR_INTF_SEL(intf_id)); > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2462 if (ret) > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2463 dev_err(dwc->dev, "function remote wakeup failed, ret:%d\n", ret); > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2464 > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2465 spin_unlock_irqrestore(&dwc->lock, flags); > 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 2466 pm_runtime_put_noidle(dwc->dev); > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2467 > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2468 return ret; > 92c08a84b53e5d Elson Roy Serrao 2023-03-24 2469 } > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-12 17:18 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-11 17:48 [PATCH 0/2] Allow dwc3 runtime suspend during bus suspend event Elson Roy Serrao 2023-05-11 17:48 ` [PATCH 1/2] usb: function: u_ether: Handle rx requests during suspend/resume Elson Roy Serrao 2023-05-11 19:33 ` kernel test robot 2023-05-11 17:48 ` [PATCH 2/2] usb: dwc3: Modify runtime pm ops to handle bus suspend Elson Roy Serrao 2023-05-12 4:18 ` Dan Carpenter 2023-05-12 17:18 ` Elson Serrao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®