* [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
* [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 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
* 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®