usb: offload: Decouple interrupter lifecycle and refactor usage tracking
Subsystems (4)
v3 cover usb: offload: Decouple interrupter lifecycle and refactor usage tracking
The current USB offload implementation couples the allocation of xHCI sideband interrupters with the device's offload usage counter. This coupling is conceptually incorrect, as hardware resource availability and power management state serve distinct purposes.
Furthermore, the reliance on the coarse USB device lock for offload state updates has led to potential recursive locking issues, especially during device disconnect when the lock is already held by the USB core.
This series refactors the offload synchronization by introducing a dedicated spinlock for offload state, allowing for more granular concurrency control and avoiding deadlocks. It also optimizes power management by ensuring that offload state is only modified when the device is already active, avoiding unnecessary auto-resumes.
Patch 1 introduces the offload_lock spinlock and offload_pm_locked synchronization, replacing the coarse udev->lock and the legacy offload_at_suspend flag. It also updates usb_offload_get/put to use pm_runtime_get_if_active().
Patch 2 removes the implicit usage tracking from the xHCI sideband layer and delegates the responsibility to class drivers, who have the correct context for managing offload data stream activity.
---
Changes in v3:
- Replace the coarse USB device lock with a dedicated offload_lock spinlock to reduce contention and prevent recursive locking.
- Introduce offload_pm_locked to synchronize with PM transitions and replace the legacy offload_at_suspend flag.
- Optimize usb_offload_get/put by switching from auto-resume/suspend to pm_runtime_get_if_active(), avoiding unnecessary power transitions.
- Explicitly delegate offload_usage tracking to USB class drivers (e.g., the Qualcomm USB audio offload driver).
- Link to v2: https://lore.kernel.org/all/[email protected]/
Changes in v2:
- Collect the <Tested-by> tag from the OPPO team
- Link to v1: https://lore.kernel.org/all/[email protected]/
Changes in v1:
- Fix build error when building sound/usb/qcom/qc_audio_offload.o
- Link to RFC v2: https://lore.kernel.org/all/[email protected]/
Changes in RFC v2:
- Move device locking to callers
- Decouple sideband from offload counting.
- Link to RFC v1: https://lore.kernel.org/all/[email protected]/
---
Guan-Yu Lin (2):
usb: core: use dedicated spinlock for offload state
usb: host: xhci-sideband: delegate offload_usage tracking to class
drivers
drivers/usb/core/driver.c | 23 ++++--- drivers/usb/core/offload.c | 107 ++++++++++++++++++------------ drivers/usb/core/usb.c | 1 + drivers/usb/host/xhci-sideband.c | 18 +---- include/linux/usb.h | 10 ++- sound/usb/qcom/qc_audio_offload.c | 10 ++- 6 files changed, 99 insertions(+), 70 deletions(-)
Signature · 1 line
2.53.0.1018.g2bb0e51243-goog
PATCH v3 1/2 usb: core: use dedicated spinlock for offload state
Replace the coarse USB device lock with a dedicated offload_lock spinlock to reduce contention during offload operations. Use offload_pm_locked to synchronize with PM transitions and replace the legacy offload_at_suspend flag.
Optimize usb_offload_get/put by switching from auto-resume/suspend to pm_runtime_get_if_active(). This ensures offload state is only modified when the device is already active, avoiding unnecessary power transitions.
--- drivers/usb/core/driver.c | 23 ++++--- drivers/usb/core/offload.c | 107 ++++++++++++++++++------------- drivers/usb/core/usb.c | 1 + drivers/usb/host/xhci-sideband.c | 4 +- include/linux/usb.h | 10 ++- 5 files changed, 89 insertions(+), 56 deletions(-)
drivers/usb/core/driver.c +14 -9
@@ -1415,14 +1415,16 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)int status =0 ;int i =0 , n =0 ;struct usb_interface *intf; +bool offload_active = false;if (udev->state ==USB_STATE_NOTATTACHED || udev->state ==USB_STATE_SUSPENDED ) goto done; +usb_offload_set_pm_locked (udev, true);if (msg.event ==PM_EVENT_SUSPEND &&usb_offload_check (udev)) {dev_dbg (&udev->dev,"device offloaded, skip suspend.\n" ); - udev->offload_at_suspend =1 ; + offload_active = true; }/* Suspend all the interfaces and then udev itself */
@@ -1436,8 +1438,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)* interrupt urbs, allowing interrupt events to be * handled during system suspend. */ -if (udev->offload_at_suspend && - intf->needs_remote_wakeup) { +if (offload_active && intf->needs_remote_wakeup) {dev_dbg (&intf->dev,"device offloaded, skip suspend.\n" );continue ;
@@ -1452,7 +1453,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg) } }if (status ==0 ) { -if (!udev->offload_at_suspend) +if (!offload_active) status =usb_suspend_device (udev, msg);/*
@@ -1498,7 +1499,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)*/ }else { udev->can_submit =0 ; -if (!udev->offload_at_suspend) { +if (!offload_active) {for (i =0 ; i <16 ; ++i) {usb_hcd_flush_endpoint (udev, udev->ep_out[i]);usb_hcd_flush_endpoint (udev, udev->ep_in[i]);
@@ -1507,6 +1508,8 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg) } done: +if (status !=0 ) +usb_offload_set_pm_locked (udev, false);dev_vdbg (&udev->dev,"%s: status %d\n" , __func__, status);return status; }
@@ -1536,16 +1539,19 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg)int status =0 ;int i;struct usb_interface *intf; +bool offload_active = false;if (udev->state ==USB_STATE_NOTATTACHED ) { status = -ENODEV ; goto done; } udev->can_submit =1 ; +if (msg.event ==PM_EVENT_RESUME ) + offload_active =usb_offload_check (udev);/* Resume the device */ if (udev->state ==USB_STATE_SUSPENDED || udev->reset_resume) { -if (!udev->offload_at_suspend) +if (!offload_active) status =usb_resume_device (udev, msg);else dev_dbg(&udev->dev,
@@ -1562,8 +1568,7 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg)* pending interrupt urbs, allowing interrupt events * to be handled during system suspend. */ -if (udev->offload_at_suspend && - intf->needs_remote_wakeup) { +if (offload_active && intf->needs_remote_wakeup) {dev_dbg (&intf->dev,"device offloaded, skip resume.\n" );continue ;
@@ -1572,11 +1577,11 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg) udev->reset_resume); } } - udev->offload_at_suspend =0 ;usb_mark_last_busy (udev); done:dev_vdbg (&udev->dev,"%s: status %d\n" , __func__, status); +usb_offload_set_pm_locked (udev, false);if (!status) udev->reset_resume =0 ;return status;
drivers/usb/core/offload.c +64 -43
157 lines not on this page · raw message
drivers/usb/core/usb.c +1 -0
@@ -671,6 +671,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,set_dev_node (&dev->dev,dev_to_node (bus->sysdev)); dev->state =USB_STATE_ATTACHED ; dev->lpm_disable_count =1 ; +spin_lock_init (&dev->offload_lock); dev->offload_usage =0 ;atomic_set (&dev->urbnum,0 );
drivers/usb/host/xhci-sideband.c +2 -2
@@ -291,8 +291,8 @@ EXPORT_SYMBOL_GPL(xhci_sideband_get_event_buffer);* Allow other drivers, such as usb controller driver, to check if there are * any sideband activity on the host controller. This information could be used * for power management or other forms of resource management. The caller should -* ensure downstream usb devices are all either suspended or marked as -* "offload_at_suspend" to ensure the correctness of the return value. +* ensure downstream usb devices are all marked as "offload_pm_locked" to ensure +* the correctness of the return value. * * Returns true on any active sideband existence, false otherwise. */
include/linux/usb.h +8 -2
@@ -21,6 +21,7 @@#include <linux/completion.h> /* for struct completion */ #include <linux/sched.h> /* for current && schedule_timeout */ #include <linux/mutex.h> /* for struct mutex */ +#include <linux/spinlock.h> /* for spinlock_t */ #include <linux/pm_runtime.h> /* for runtime PM */ struct usb_device ;
@@ -636,8 +637,9 @@ struct usb3_lpm_parameters {* @do_remote_wakeup: remote wakeup should be enabled * @reset_resume: needs reset instead of resume * @port_is_suspended: the upstream port is suspended (L2 or U3) -* @offload_at_suspend: offload activities during suspend is enabled. +* @offload_pm_locked: prevents offload_usage changes during PM transitions. * @offload_usage: number of offload activities happening on this usb device. +* @offload_lock: protects offload_usage and offload_pm_locked * @slot_id: Slot ID assigned by xHCI * @l1_params: best effor service latency for USB2 L1 LPM state, and L1 timeout. * @u1_params: exit latencies for USB3 U1 LPM state, and hub-initiated timeout.
@@ -726,8 +728,9 @@ struct usb_device { unsigned do_remote_wakeup:1; unsigned reset_resume:1; unsigned port_is_suspended:1; - unsigned offload_at_suspend:1; + unsigned offload_pm_locked:1;int offload_usage; +spinlock_t offload_lock;enum usb_link_tunnel_mode tunnel_mode;struct device_link *usb4_link;
@@ -849,6 +852,7 @@ static inline void usb_mark_last_busy(struct usb_device *udev)int usb_offload_get (struct usb_device *udev);int usb_offload_put (struct usb_device *udev);bool usb_offload_check (struct usb_device *udev); +void usb_offload_set_pm_locked (struct usb_device *udev,bool locked);#else static inline int usb_offload_get (struct usb_device *udev)
@@ -857,6 +861,8 @@ static inline int usb_offload_put(struct usb_device *udev) {return 0 ; }static inline bool usb_offload_check (struct usb_device *udev) {return false; } +static inline void usb_offload_set_pm_locked (struct usb_device *udev,bool locked) +{ }#endif extern int usb_disable_lpm (struct usb_device *udev);
Signature · 1 line
2.53.0.1018.g2bb0e51243-goog
Re: [PATCH v3 1/2] usb: core: use dedicated spinlock for offload state
Hi
Quoted 43 lines · Guan-Yu Lin
Replace the coarse USB device lock with a dedicated
offload_lockspinlock to reduce contention during offload operations. Useoffload_pm_lockedto synchronize with PM transitions and replace the legacyoffload_at_suspendflag.Optimize usb_offload_get/put by switching from auto-resume/suspend to
pm_runtime_get_if_active(). This ensures offload state is only modified when the device is already active, avoiding unnecessary power transitions.Fixes ef82a4803aab ("xhci: sideband: add api to trace sideband usage")Signed-off-by Guan-Yu Lin <[email protected]>---
drivers/usb/core/offload.c +5
-10@@ -25,33 +25,30 @@*/ int usb_offload_get (struct usb_device *udev) { -int ret; +int ret =0 ; -usb_lock_device (udev); -if (udev->state ==USB_STATE_NOTATTACHED ) { -usb_unlock_device (udev); +if (!usb_get_dev (udev))return -ENODEV ; - } -if (udev->state ==USB_STATE_SUSPENDED || - udev->offload_at_suspend) { -usb_unlock_device (udev); -return -EBUSY ; +if (pm_runtime_get_if_active (&udev->dev) !=1 ) { + ret = -EBUSY ; + goto err_rpm; } -/*
drivers/usb/core/offload.c line 40
Quoted 10 lines
drivers/usb/core/offload.c +3
-7-* offload_usage could only be modified when the device is active, since -* it will alter the suspend flow of the device. -*/ - ret =usb_autoresume_device (udev); -if (ret <0 ) { -usb_unlock_device (udev); -return ret; +spin_lock (&udev->offload_lock); + +if (udev->offload_pm_locked) {
Could we get rid of 'udev->offload_pm_locked' and 'usb_offload_set_pm_locked()' by calling a synchronous pm_runtime_get_sync() or pm_runtime_resume_and_get()?
This way we can ensure udev->offload_usage isn't modified mid runtime suspend or resume as resume is guaranteed to have finished and suspend won't be called, at least not for the runtime case.
Quoted 42 lines
drivers/usb/core/offload.c +13
-14+ ret = -EAGAIN ; + goto err; } udev->offload_usage++; -usb_autosuspend_device (udev); -usb_unlock_device (udev); + +err: +spin_unlock (&udev->offload_lock); +pm_runtime_put_autosuspend (&udev->dev); +err_rpm: +usb_put_dev (udev);return ret; }@@ -69,35 +66,32 @@ EXPORT_SYMBOL_GPL(usb_offload_get);*/ int usb_offload_put (struct usb_device *udev) { -int ret; +int ret =0 ; -usb_lock_device (udev); -if (udev->state ==USB_STATE_NOTATTACHED ) { -usb_unlock_device (udev); +if (!usb_get_dev (udev))return -ENODEV ; - } -if (udev->state ==USB_STATE_SUSPENDED || - udev->offload_at_suspend) { -usb_unlock_device (udev); -return -EBUSY ; +if (pm_runtime_get_if_active (&udev->dev) !=1 ) { + ret = -EBUSY ; + goto err_rpm; } -/* -* offload_usage could only be modified when the device is active, since -* it will alter the suspend flow of the device.
drivers/usb/core/offload.c EXPORT_SYMBOL_GPL(usb_offload_get); · line 83
Quoted 10 lines
drivers/usb/core/offload.c +5
-5-*/ - ret =usb_autoresume_device (udev); -if (ret <0 ) { -usb_unlock_device (udev); -return ret; +spin_lock (&udev->offload_lock); + +if (udev->offload_pm_locked) { + ret = -EAGAIN ; + goto err;
Ending up here is about unlucky timing, i.e. usb_offload_put() is called while device is pretending to suspend/resume. Result here is that udev->offload_usage is not decremented, and usb device won't properly suspend anymore even if device is no longer offloaded.
Quoted 30 lines
drivers/usb/core/offload.c +8
-4}/* Drop the count when it wasn't 0, ignore the operation otherwise. */ if (udev->offload_usage) udev->offload_usage--; -usb_autosuspend_device (udev); -usb_unlock_device (udev); + +err: +spin_unlock (&udev->offload_lock); +pm_runtime_put_autosuspend (&udev->dev); +err_rpm: +usb_put_dev (udev);return ret; }@@ -112,25 +106,52 @@ EXPORT_SYMBOL_GPL(usb_offload_put);* management. * * The caller must hold @udev's device lock. In addition, the caller should -* ensure downstream usb devices are all either suspended or marked as -* "offload_at_suspend" to ensure the correctness of the return value. +* ensure downstream usb devices are all marked as "offload_pm_locked" to +* ensure the correctness of the return value. * * Returns true on any offload activity, false otherwise. */ bool usb_offload_check(struct usb_device *udev) __must_hold(&udev->dev->mutex) {struct usb_device *child;
drivers/usb/core/offload.c EXPORT_SYMBOL_GPL(usb_offload_put); · line 125
Quoted 10 lines
drivers/usb/core/offload.c +7
-1-bool active; +bool active = false;int port1; +spin_lock (&udev->offload_lock); +if (udev->offload_usage) + active = true; +spin_unlock (&udev->offload_lock); +> + if (active) +return true;
Not sure what the purpose of the spinlock is above
Quoted 22 lines
drivers/usb/core/offload.c +10
-2+usb_hub_for_each_child (udev, port1, child) {usb_lock_device (child); active =usb_offload_check (child);usb_unlock_device (child); +if (active) -return true; +break ; } -return !!udev->offload_usage; +return active; }EXPORT_SYMBOL_GPL (usb_offload_check); + +/** +* usb_offload_set_pm_locked - set the PM lock state of a USB device +* @udev: the USB device to modify +* @locked: the new lock state +*
drivers/usb/core/offload.c EXPORT_SYMBOL_GPL(usb_offload_put); · line 155
Quoted 10 lines
drivers/usb/core/offload.c +10
-0+* Setting @locked to true prevents offload_usage from being modified. This +* ensures that offload activities cannot be started or stopped during critical +* power management transitions, maintaining a stable state for the duration +* of the transition. +*/ +void usb_offload_set_pm_locked (struct usb_device *udev,bool locked) +{ +spin_lock (&udev->offload_lock); + udev->offload_pm_locked = locked; +spin_unlock (&udev->offload_lock);
spinlock usage unclear here as well
Thanks
Mathias
Re: [PATCH v3 1/2] usb: core: use dedicated spinlock for offload state
Quoted 16 lines · Mathias Nyman
Hi
drivers/usb/core/offload.c line 40
Quoted 11 lines · Guan-Yu Lin
drivers/usb/core/offload.c +3
-8-/* -* offload_usage could only be modified when the device is active, since -* it will alter the suspend flow of the device. -*/ - ret =usb_autoresume_device (udev); -if (ret <0 ) { -usb_unlock_device (udev); -return ret; +spin_lock (&udev->offload_lock); + +if (udev->offload_pm_locked) {
Quoted 7 lines
Could we get rid of '
udev->offload_pm_locked' and 'usb_offload_set_pm_locked()' by calling a synchronouspm_runtime_get_sync()orpm_runtime_resume_and_get()?This way we can ensure
udev->offload_usageisn't modified mid runtime suspend or resume as resume is guaranteed to have finished and suspend won't be called, at least not for the runtime case.
We previously considered manipulating runtime PM directly, but the USB-specific runtime API (usb_autoresume_device()) necessitates holding the device lock. Since holding this lock causes a deadlock issue as reported, we implemented a check-and-return-error approach to remain deadlock-free while staying consistent with USB locking requirements.
The offload_pm_locked flag ensures that offload_usage remains static throughout the system suspend/resume cycle. Because modifications to usage counts directly impact the USB power management flow, all updates must occur only when the device is fully active.
Quoted 15 lines
drivers/usb/core/offload.c line 42
Quoted 13 lines
drivers/usb/core/offload.c +5
-8-/* -* offload_usage could only be modified when the device is active, since -* it will alter the suspend flow of the device. -*/ - ret =usb_autoresume_device (udev); -if (ret <0 ) { -usb_unlock_device (udev); -return ret; +spin_lock (&udev->offload_lock); + +if (udev->offload_pm_locked) { + ret = -EAGAIN ; + goto err;
Quoted 5 lines
Ending up here is about unlucky timing, i.e.
usb_offload_put()is called while device is pretending to suspend/resume. Result here is thatudev->offload_usageis not decremented, and usb device won't properly suspend anymore even if device is no longer offloaded.
The current implementation requires the class driver to either handle the error code properly or ensure the system is fully resumed from system suspend before calling usb_offload_get()/put().
If we use the idea from pm_runtime_put_sync(): "decremented usage count in all cases, even if it returns an error code", this will lead to usage_count changes during system suspend. The system might use suspend logic with active USB offloaded devices system to suspend, but uses logic without active offloaded devices since the usage count has been changed during the suspend period. To avoid this, a check-and-return-error approach is adapted here.
Quoted 7 lines
Quoted 7 lines
bool usb_offload_check(struct usb_device *udev) __must_hold(&udev->dev->mutex) { struct usb_device *child;drivers/usb/core/offload.c +1
-1-bool active; +bool active = false;int port1;
Quoted 9 lines
drivers/usb/core/offload.c EXPORT_SYMBOL_GPL(usb_offload_put); · line 126
Quoted 6 lines
drivers/usb/core/offload.c +6
-0+spin_lock (&udev->offload_lock); +if (udev->offload_usage) + active = true; +spin_unlock (&udev->offload_lock); +> + if (active) +return true;Not sure what the purpose of the spinlock is above
The spinlock was originally intended to prevent a race condition between usb_offload_get()/put() and usb_offload_check(). However, because usb_offload_check() is only invoked while offload_pm_locked is set, the race is already resolved. I’m removing the spinlock and updating the function description accordingly. Thanks for identifying this.
Quoted 9 lines
drivers/usb/core/offload.c EXPORT_SYMBOL_GPL(usb_offload_put); · line 155
Quoted 6 lines
drivers/usb/core/offload.c +5
-0+void usb_offload_set_pm_locked (struct usb_device *udev,bool locked) +{ +spin_lock (&udev->offload_lock); + udev->offload_pm_locked = locked; +spin_unlock (&udev->offload_lock);spinlock usage unclear here as well
Quoted 3 lines
Thanks
Mathias
The spinlock ensures that offload_usage is modified only while offload_pm_locked remains false. Without this synchronization, offload_pm_locked could change during the execution of usb_offload_get()/put(), leading to a race condition.
Regards,
Guan-Yu
PATCH v3 2/2 usb: host: xhci-sideband: delegate offload_usage tracking to class drivers
In reply to Guan-Yu LinRemove usb_offload_get() and usb_offload_put() from the xHCI sideband interrupter creation and removal paths.
The responsibility of manipulating offload_usage now lies entirely with the USB class drivers. They have the precise context of when an offload data stream actually starts and stops, ensuring a much more accurate representation of offload activity for power management.
--- drivers/usb/host/xhci-sideband.c | 14 +------------- sound/usb/qcom/qc_audio_offload.c | 10 +++++++++- 2 files changed, 10 insertions(+), 14 deletions(-)
drivers/usb/host/xhci-sideband.c +1 -13
@@ -93,8 +93,6 @@ __xhci_sideband_remove_endpoint(struct xhci_sideband *sb, struct xhci_virt_ep *estatic void __xhci_sideband_remove_interrupter (struct xhci_sideband *sb) { -struct usb_device *udev; -lockdep_assert_held (&sb->mutex);if (!sb->ir)
@@ -102,10 +100,6 @@ __xhci_sideband_remove_interrupter(struct xhci_sideband *sb)xhci_remove_secondary_interrupter (xhci_to_hcd (sb->xhci), sb->ir); sb->ir =NULL ; - udev = sb->vdev->udev; - -if (udev->state !=USB_STATE_NOTATTACHED ) -usb_offload_put (udev); }/* sideband api functions */
@@ -328,9 +322,6 @@ int xhci_sideband_create_interrupter(struct xhci_sideband *sb, int num_seg, bool ip_autoclear, u32 imod_interval, int intr_num) { - int ret = 0; -struct usb_device *udev; - if (!sb || !sb->xhci) return -ENODEV;
@@ -348,12 +339,9 @@ xhci_sideband_create_interrupter(struct xhci_sideband *sb, int num_seg,if (!sb->ir)return -ENOMEM ; - udev = sb->vdev->udev; - ret =usb_offload_get (udev); - sb->ir->ip_autoclear = ip_autoclear; -return ret; +return 0 ; }EXPORT_SYMBOL_GPL (xhci_sideband_create_interrupter);
sound/usb/qcom/qc_audio_offload.c +9 -1
@@ -699,6 +699,7 @@ static void uaudio_event_ring_cleanup_free(struct uaudio_dev *dev)uaudio_iommu_unmap (MEM_EVENT_RING ,IOVA_BASE ,PAGE_SIZE ,PAGE_SIZE );xhci_sideband_remove_interrupter (uadev[dev->chip->card->number].sb); +usb_offload_put (dev->udev); } }
@@ -1182,12 +1183,16 @@ static int uaudio_event_ring_setup(struct snd_usb_substream *subs, dma_coherent =dev_is_dma_coherent (subs->dev->bus->sysdev); er_pa =0 ; + ret =usb_offload_get (subs->dev); +if (ret <0 ) + goto exit; +/* event ring */ ret =xhci_sideband_create_interrupter (uadev[card_num].sb,1 , false,0 , uaudio_qdev->data->intr_num);if (ret <0 ) {dev_err (&subs->dev->dev,"failed to fetch interrupter\n" ); - goto exit; + goto put_offload; } sgt =xhci_sideband_get_event_buffer (uadev[card_num].sb);
@@ -1219,6 +1224,8 @@ static int uaudio_event_ring_setup(struct snd_usb_substream *subs, mem_info->dma =0 ; remove_interrupter:xhci_sideband_remove_interrupter (uadev[card_num].sb); +put_offload: +usb_offload_put (subs->dev); exit:return ret; }
@@ -1483,6 +1490,7 @@ static int prepare_qmi_response(struct snd_usb_substream *subs,uaudio_iommu_unmap (MEM_EVENT_RING ,IOVA_BASE ,PAGE_SIZE ,PAGE_SIZE ); free_sec_ring:xhci_sideband_remove_interrupter (uadev[card_num].sb); +usb_offload_put (subs->dev); drop_sync_ep:if (subs->sync_endpoint) { uaudio_iommu_unmap(MEM_XFER_RING,
Signature · 1 line
2.53.0.1018.g2bb0e51243-goog
Re [PATCH v3 0/2] usb: offload: Decouple interrupter lifecycle and refactor usage tracking
In reply to Guan-Yu LinHi,
We, OPPO kernel team, have thoroughly verified this patch on our devices. The test results confirm that it effectively resolves a critical headset hot-plugging issue and the subsequent system deadlock.
Impact:
This issue is currently a blocker for our upcoming product shipping. We have observed a high reproduction rate on our latest platforms, and this patch is essential for our production stability.
Test Environment:
Devices: OPPO devices based on SM8850 and SM8845 platforms. OS/Kernel: Android 16.0 / Linux Kernel 6.12.
Background:
High reproduction rate (typically within 10 cycles) before applying this patch.
Original Reproduction Path (Verified fixed):
Open music player and enable USB exclusive mode. Connect a Type-C digital headset and start playback. Repeatedly plug/unplug the headset.
Observation:
Within 10 cycles, the headset icon would persist after unplugging, followed by no audio upon reconnection and a system deadlock.
Stress Test Results with This Patch (Phone A & B):
Test Case 1: USB Exclusive Mode Hot-plug
Steps: Enable USB exclusive mode -> Play music -> Plug/unplug 100 cycles.
Result: PASS. Audio functions normally; no deadlock observed.
Test Case 2: Standard Mode Hot-plug
Steps: Disable USB exclusive mode -> Play music -> Plug/unplug 100 cycles.
Result: PASS. Audio and system stability remain normal.
Test Case 3: Mixed Accessory Switching (Headset/Charger)
Steps: Play music -> Unplug headset -> Plug in charger -> Unplug charger -> Re-plug headset. Repeated for 30 cycles.
Result: PASS. No issues observed.
The patch looks solid and is critical for our project. We hope these results help expedite the upstreaming process.
Free to add