[PATCH v1 1/2] usb: offload: move device locking to callers in offload.c

[PATCH v1 1/2] usb: offload: move device locking to callers in offload.c

[Guan-Yu Lin]

Update usb_offload_get() and usb_offload_put() to require that the
caller holds the USB device lock. Remove the internal call to
usb_lock_device() and add device_lock_assert() to ensure synchronization
is handled by the caller. These functions continue to manage the
device's power state via autoresume/autosuspend and update the
offload_usage counter.

Additionally, decouple the xHCI sideband interrupter lifecycle from the
offload usage counter by removing the calls to usb_offload_get() and
usb_offload_put() from the interrupter creation and removal paths. This
allows interrupters to be managed independently of the device's offload
activity status.

Cc: stable@vger.kernel.org
Fixes: ef82a4803aab ("xhci: sideband: add api to trace sideband usage")
Signed-off-by: Guan-Yu Lin <guanyulin@google.com>
---
 drivers/usb/core/offload.c       | 34 +++++++++++---------------------
 drivers/usb/host/xhci-sideband.c | 14 +------------
 2 files changed, 13 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/core/offload.c b/drivers/usb/core/offload.c
index 7c699f1b8d2b..e13a4c21d61b 100644
--- a/drivers/usb/core/offload.c
+++ b/drivers/usb/core/offload.c
@@ -20,6 +20,7 @@
  * enabled on this usb_device; that is, another entity is actively handling USB
  * transfers. This information allows the USB driver to adjust its power
  * management policy based on offload activity.
+ * The caller must hold @udev's device lock.
  *
  * Return: 0 on success. A negative error code otherwise.
  */
@@ -27,31 +28,25 @@ int usb_offload_get(struct usb_device *udev)
 {
 	int ret;
 
-	usb_lock_device(udev);
-	if (udev->state == USB_STATE_NOTATTACHED) {
-		usb_unlock_device(udev);
+	device_lock_assert(&udev->dev);
+
+	if (udev->state == USB_STATE_NOTATTACHED)
 		return -ENODEV;
-	}
 
 	if (udev->state == USB_STATE_SUSPENDED ||
-		   udev->offload_at_suspend) {
-		usb_unlock_device(udev);
+	    udev->offload_at_suspend)
 		return -EBUSY;
-	}
 
 	/*
 	 * 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);
+	if (ret < 0)
 		return ret;
-	}
 
 	udev->offload_usage++;
 	usb_autosuspend_device(udev);
-	usb_unlock_device(udev);
 
 	return ret;
 }
@@ -64,6 +59,7 @@ EXPORT_SYMBOL_GPL(usb_offload_get);
  * The inverse operation of usb_offload_get, which drops the offload_usage of
  * a USB device. This information allows the USB driver to adjust its power
  * management policy based on offload activity.
+ * The caller must hold @udev's device lock.
  *
  * Return: 0 on success. A negative error code otherwise.
  */
@@ -71,33 +67,27 @@ int usb_offload_put(struct usb_device *udev)
 {
 	int ret;
 
-	usb_lock_device(udev);
-	if (udev->state == USB_STATE_NOTATTACHED) {
-		usb_unlock_device(udev);
+	device_lock_assert(&udev->dev);
+
+	if (udev->state == USB_STATE_NOTATTACHED)
 		return -ENODEV;
-	}
 
 	if (udev->state == USB_STATE_SUSPENDED ||
-		   udev->offload_at_suspend) {
-		usb_unlock_device(udev);
+	    udev->offload_at_suspend)
 		return -EBUSY;
-	}
 
 	/*
 	 * 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);
+	if (ret < 0)
 		return ret;
-	}
 
 	/* 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);
 
 	return ret;
 }
diff --git a/drivers/usb/host/xhci-sideband.c b/drivers/usb/host/xhci-sideband.c
index 2bd77255032b..6fc0ad658d66 100644
--- a/drivers/usb/host/xhci-sideband.c
+++ b/drivers/usb/host/xhci-sideband.c
@@ -93,8 +93,6 @@ __xhci_sideband_remove_endpoint(struct xhci_sideband *sb, struct xhci_virt_ep *e
 static 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);
 
-- 
2.53.0.414.gf7e9f6c205-goog

Re [PATCH v1 1/2] usb: offload: move device locking to callers in offload.c

[hailong]

Our OPPO team has tested this patch on an qcom8850 OPD2514 OnePlus
Pad 4 (arm64), based on Android 16 kernel 6.12.

Before adding this patch, the issue was always present after 100
electrostatic discharge tests on the headphone jack. With this patch,
the problem has been fixed on both tested devices. We hope this can
be merged into the final product list.

Tested-by: Hailong Liu <hailong.liu@oppo.com>

[PATCH v1 2/2] ALSA: usb: qcom: manage offload device usage

[Guan-Yu Lin]

The Qualcomm USB audio offload driver currently does not report its offload
activity to the USB core. This prevents the USB core from properly tracking
active offload sessions, which could allow the device to auto-suspend while
audio offloading is in progress.

Integrate usb_offload_get() and usb_offload_put() calls into the offload
stream setup and teardown paths. Specifically, call usb_offload_get() when
initializing the event ring and usb_offload_put() when freeing it.

Since the updated usb_offload_get() and usb_offload_put() APIs require the
caller to hold the USB device lock, add the necessary device locking in
handle_uaudio_stream_req() and qmi_stop_session() to satisfy this
requirement.

Cc: stable@vger.kernel.org
Fixes: ef82a4803aab ("xhci: sideband: add api to trace sideband usage")
Signed-off-by: Guan-Yu Lin <guanyulin@google.com>
---
 sound/usb/qcom/qc_audio_offload.c | 102 ++++++++++++++++++------------
 1 file changed, 60 insertions(+), 42 deletions(-)

diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
index cfb30a195364..1da243662327 100644
--- a/sound/usb/qcom/qc_audio_offload.c
+++ b/sound/usb/qcom/qc_audio_offload.c
@@ -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);
 	}
 }
 
@@ -750,6 +751,7 @@ static void qmi_stop_session(void)
 	struct snd_usb_substream *subs;
 	struct usb_host_endpoint *ep;
 	struct snd_usb_audio *chip;
+	struct usb_device *udev;
 	struct intf_info *info;
 	int pcm_card_num;
 	int if_idx;
@@ -791,8 +793,13 @@ static void qmi_stop_session(void)
 			disable_audio_stream(subs);
 		}
 		atomic_set(&uadev[idx].in_use, 0);
-		guard(mutex)(&chip->mutex);
-		uaudio_dev_cleanup(&uadev[idx]);
+
+		udev = uadev[idx].udev;
+		if (udev) {
+			guard(device)(&udev->dev);
+			guard(mutex)(&chip->mutex);
+			uaudio_dev_cleanup(&uadev[idx]);
+		}
 	}
 }
 
@@ -1183,11 +1190,15 @@ static int uaudio_event_ring_setup(struct snd_usb_substream *subs,
 	er_pa = 0;
 
 	/* event ring */
+	ret = usb_offload_get(subs->dev);
+	if (ret < 0)
+		goto exit;
+
 	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 +1230,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 +1496,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,
@@ -1528,6 +1542,7 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
 	u8 pcm_card_num;
 	u8 pcm_dev_num;
 	u8 direction;
+	struct usb_device *udev = NULL;
 	int ret = 0;
 
 	if (!svc->client_connected) {
@@ -1597,50 +1612,53 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
 
 	uadev[pcm_card_num].ctrl_intf = chip->ctrl_intf;
 
-	if (req_msg->enable) {
-		ret = enable_audio_stream(subs,
-					  map_pcm_format(req_msg->audio_format),
-					  req_msg->number_of_ch, req_msg->bit_rate,
-					  datainterval);
-
-		if (!ret)
-			ret = prepare_qmi_response(subs, req_msg, &resp,
-						   info_idx);
-		if (ret < 0) {
-			guard(mutex)(&chip->mutex);
-			subs->opened = 0;
-		}
-	} else {
-		info = &uadev[pcm_card_num].info[info_idx];
-		if (info->data_ep_pipe) {
-			ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
-					       info->data_ep_pipe);
-			if (ep) {
-				xhci_sideband_stop_endpoint(uadev[pcm_card_num].sb,
-							    ep);
-				xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb,
-							      ep);
+	udev = subs->dev;
+	scoped_guard(device, &udev->dev) {
+		if (req_msg->enable) {
+			ret = enable_audio_stream(subs,
+						map_pcm_format(req_msg->audio_format),
+						req_msg->number_of_ch, req_msg->bit_rate,
+						datainterval);
+
+			if (!ret)
+				ret = prepare_qmi_response(subs, req_msg, &resp,
+							info_idx);
+			if (ret < 0) {
+				guard(mutex)(&chip->mutex);
+				subs->opened = 0;
+			}
+		} else {
+			info = &uadev[pcm_card_num].info[info_idx];
+			if (info->data_ep_pipe) {
+				ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
+							info->data_ep_pipe);
+				if (ep) {
+					xhci_sideband_stop_endpoint(uadev[pcm_card_num].sb,
+									ep);
+					xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb,
+									ep);
+				}
+
+				info->data_ep_pipe = 0;
 			}
 
-			info->data_ep_pipe = 0;
-		}
-
-		if (info->sync_ep_pipe) {
-			ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
-					       info->sync_ep_pipe);
-			if (ep) {
-				xhci_sideband_stop_endpoint(uadev[pcm_card_num].sb,
-							    ep);
-				xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb,
-							      ep);
+			if (info->sync_ep_pipe) {
+				ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
+							info->sync_ep_pipe);
+				if (ep) {
+					xhci_sideband_stop_endpoint(uadev[pcm_card_num].sb,
+									ep);
+					xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb,
+									ep);
+				}
+
+				info->sync_ep_pipe = 0;
 			}
 
-			info->sync_ep_pipe = 0;
+			disable_audio_stream(subs);
+			guard(mutex)(&chip->mutex);
+			subs->opened = 0;
 		}
-
-		disable_audio_stream(subs);
-		guard(mutex)(&chip->mutex);
-		subs->opened = 0;
 	}
 
 response:
-- 
2.53.0.414.gf7e9f6c205-goog

Re: [PATCH v1 2/2] ALSA: usb: qcom: manage offload device usage

[Takashi Iwai]

On Wed, 25 Feb 2026 07:45:51 +0100,
Guan-Yu Lin wrote:
> 
> The Qualcomm USB audio offload driver currently does not report its offload
> activity to the USB core. This prevents the USB core from properly tracking
> active offload sessions, which could allow the device to auto-suspend while
> audio offloading is in progress.
> 
> Integrate usb_offload_get() and usb_offload_put() calls into the offload
> stream setup and teardown paths. Specifically, call usb_offload_get() when
> initializing the event ring and usb_offload_put() when freeing it.
> 
> Since the updated usb_offload_get() and usb_offload_put() APIs require the
> caller to hold the USB device lock, add the necessary device locking in
> handle_uaudio_stream_req() and qmi_stop_session() to satisfy this
> requirement.
> 
> Cc: stable@vger.kernel.org
> Fixes: ef82a4803aab ("xhci: sideband: add api to trace sideband usage")
> Signed-off-by: Guan-Yu Lin <guanyulin@google.com>

Mostly it looks good to me, but a slight concern is the
usb_offload_put() call in the error path of prepare_qmi_response().
IIUC, the bitmap is cleared only at uaudio_event_ring_cleanup_free(),
and you have also the usb_offload_put() call there.  I wonder whether
this might lead to the refcount unbalance.

I'm not sure whether the original driver code handles it well, and
this could be already a bug there calling
xhci_sideband_remove_interrupter(), though.


thanks,

Takashi

> ---
>  sound/usb/qcom/qc_audio_offload.c | 102 ++++++++++++++++++------------
>  1 file changed, 60 insertions(+), 42 deletions(-)
> 
> diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
> index cfb30a195364..1da243662327 100644
> --- a/sound/usb/qcom/qc_audio_offload.c
> +++ b/sound/usb/qcom/qc_audio_offload.c
> @@ -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);
>  	}
>  }
>  
> @@ -750,6 +751,7 @@ static void qmi_stop_session(void)
>  	struct snd_usb_substream *subs;
>  	struct usb_host_endpoint *ep;
>  	struct snd_usb_audio *chip;
> +	struct usb_device *udev;
>  	struct intf_info *info;
>  	int pcm_card_num;
>  	int if_idx;
> @@ -791,8 +793,13 @@ static void qmi_stop_session(void)
>  			disable_audio_stream(subs);
>  		}
>  		atomic_set(&uadev[idx].in_use, 0);
> -		guard(mutex)(&chip->mutex);
> -		uaudio_dev_cleanup(&uadev[idx]);
> +
> +		udev = uadev[idx].udev;
> +		if (udev) {
> +			guard(device)(&udev->dev);
> +			guard(mutex)(&chip->mutex);
> +			uaudio_dev_cleanup(&uadev[idx]);
> +		}
>  	}
>  }
>  
> @@ -1183,11 +1190,15 @@ static int uaudio_event_ring_setup(struct snd_usb_substream *subs,
>  	er_pa = 0;
>  
>  	/* event ring */
> +	ret = usb_offload_get(subs->dev);
> +	if (ret < 0)
> +		goto exit;
> +
>  	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 +1230,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 +1496,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,
> @@ -1528,6 +1542,7 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
>  	u8 pcm_card_num;
>  	u8 pcm_dev_num;
>  	u8 direction;
> +	struct usb_device *udev = NULL;
>  	int ret = 0;
>  
>  	if (!svc->client_connected) {
> @@ -1597,50 +1612,53 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
>  
>  	uadev[pcm_card_num].ctrl_intf = chip->ctrl_intf;
>  
> -	if (req_msg->enable) {
> -		ret = enable_audio_stream(subs,
> -					  map_pcm_format(req_msg->audio_format),
> -					  req_msg->number_of_ch, req_msg->bit_rate,
> -					  datainterval);
> -
> -		if (!ret)
> -			ret = prepare_qmi_response(subs, req_msg, &resp,
> -						   info_idx);
> -		if (ret < 0) {
> -			guard(mutex)(&chip->mutex);
> -			subs->opened = 0;
> -		}
> -	} else {
> -		info = &uadev[pcm_card_num].info[info_idx];
> -		if (info->data_ep_pipe) {
> -			ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
> -					       info->data_ep_pipe);
> -			if (ep) {
> -				xhci_sideband_stop_endpoint(uadev[pcm_card_num].sb,
> -							    ep);
> -				xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb,
> -							      ep);
> +	udev = subs->dev;
> +	scoped_guard(device, &udev->dev) {
> +		if (req_msg->enable) {
> +			ret = enable_audio_stream(subs,
> +						map_pcm_format(req_msg->audio_format),
> +						req_msg->number_of_ch, req_msg->bit_rate,
> +						datainterval);
> +
> +			if (!ret)
> +				ret = prepare_qmi_response(subs, req_msg, &resp,
> +							info_idx);
> +			if (ret < 0) {
> +				guard(mutex)(&chip->mutex);
> +				subs->opened = 0;
> +			}
> +		} else {
> +			info = &uadev[pcm_card_num].info[info_idx];
> +			if (info->data_ep_pipe) {
> +				ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
> +							info->data_ep_pipe);
> +				if (ep) {
> +					xhci_sideband_stop_endpoint(uadev[pcm_card_num].sb,
> +									ep);
> +					xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb,
> +									ep);
> +				}
> +
> +				info->data_ep_pipe = 0;
>  			}
>  
> -			info->data_ep_pipe = 0;
> -		}
> -
> -		if (info->sync_ep_pipe) {
> -			ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
> -					       info->sync_ep_pipe);
> -			if (ep) {
> -				xhci_sideband_stop_endpoint(uadev[pcm_card_num].sb,
> -							    ep);
> -				xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb,
> -							      ep);
> +			if (info->sync_ep_pipe) {
> +				ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
> +							info->sync_ep_pipe);
> +				if (ep) {
> +					xhci_sideband_stop_endpoint(uadev[pcm_card_num].sb,
> +									ep);
> +					xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb,
> +									ep);
> +				}
> +
> +				info->sync_ep_pipe = 0;
>  			}
>  
> -			info->sync_ep_pipe = 0;
> +			disable_audio_stream(subs);
> +			guard(mutex)(&chip->mutex);
> +			subs->opened = 0;
>  		}
> -
> -		disable_audio_stream(subs);
> -		guard(mutex)(&chip->mutex);
> -		subs->opened = 0;
>  	}
>  
>  response:
> -- 
> 2.53.0.414.gf7e9f6c205-goog
> 

Re: [PATCH v1 2/2] ALSA: usb: qcom: manage offload device usage

[Guan-Yu Lin]

Hi Takashi,

Thanks for raising the concern. The fix I proposed aims only to
separate xhci_sideband_remove_interrupter() and usb_offload_put().
Let's wait for QC's reply to see whether this is a rigid fix.

 Regards,
 Guan-Yu

On Fri, Feb 27, 2026 at 1:13 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Wed, 25 Feb 2026 07:45:51 +0100,
> Guan-Yu Lin wrote:
> >
> > The Qualcomm USB audio offload driver currently does not report its offload
> > activity to the USB core. This prevents the USB core from properly tracking
> > active offload sessions, which could allow the device to auto-suspend while
> > audio offloading is in progress.
> >
> > Integrate usb_offload_get() and usb_offload_put() calls into the offload
> > stream setup and teardown paths. Specifically, call usb_offload_get() when
> > initializing the event ring and usb_offload_put() when freeing it.
> >
> > Since the updated usb_offload_get() and usb_offload_put() APIs require the
> > caller to hold the USB device lock, add the necessary device locking in
> > handle_uaudio_stream_req() and qmi_stop_session() to satisfy this
> > requirement.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: ef82a4803aab ("xhci: sideband: add api to trace sideband usage")
> > Signed-off-by: Guan-Yu Lin <guanyulin@google.com>
>
> Mostly it looks good to me, but a slight concern is the
> usb_offload_put() call in the error path of prepare_qmi_response().
> IIUC, the bitmap is cleared only at uaudio_event_ring_cleanup_free(),
> and you have also the usb_offload_put() call there.  I wonder whether
> this might lead to the refcount unbalance.
>
> I'm not sure whether the original driver code handles it well, and
> this could be already a bug there calling
> xhci_sideband_remove_interrupter(), though.
>
>
> thanks,
>
> Takashi
>
> > ---
> >  sound/usb/qcom/qc_audio_offload.c | 102 ++++++++++++++++++------------
> >  1 file changed, 60 insertions(+), 42 deletions(-)
> >
> > diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
> > index cfb30a195364..1da243662327 100644
> > --- a/sound/usb/qcom/qc_audio_offload.c
> > +++ b/sound/usb/qcom/qc_audio_offload.c
> > @@ -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);
> >       }
> >  }
> >
> > @@ -750,6 +751,7 @@ static void qmi_stop_session(void)
> >       struct snd_usb_substream *subs;
> >       struct usb_host_endpoint *ep;
> >       struct snd_usb_audio *chip;
> > +     struct usb_device *udev;
> >       struct intf_info *info;
> >       int pcm_card_num;
> >       int if_idx;
> > @@ -791,8 +793,13 @@ static void qmi_stop_session(void)
> >                       disable_audio_stream(subs);
> >               }
> >               atomic_set(&uadev[idx].in_use, 0);
> > -             guard(mutex)(&chip->mutex);
> > -             uaudio_dev_cleanup(&uadev[idx]);
> > +
> > +             udev = uadev[idx].udev;
> > +             if (udev) {
> > +                     guard(device)(&udev->dev);
> > +                     guard(mutex)(&chip->mutex);
> > +                     uaudio_dev_cleanup(&uadev[idx]);
> > +             }
> >       }
> >  }
> >
> > @@ -1183,11 +1190,15 @@ static int uaudio_event_ring_setup(struct snd_usb_substream *subs,
> >       er_pa = 0;
> >
> >       /* event ring */
> > +     ret = usb_offload_get(subs->dev);
> > +     if (ret < 0)
> > +             goto exit;
> > +
> >       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 +1230,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 +1496,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,
> > @@ -1528,6 +1542,7 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
> >       u8 pcm_card_num;
> >       u8 pcm_dev_num;
> >       u8 direction;
> > +     struct usb_device *udev = NULL;
> >       int ret = 0;
> >
> >       if (!svc->client_connected) {
> > @@ -1597,50 +1612,53 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
> >
> >       uadev[pcm_card_num].ctrl_intf = chip->ctrl_intf;
> >
> > -     if (req_msg->enable) {
> > -             ret = enable_audio_stream(subs,
> > -                                       map_pcm_format(req_msg->audio_format),
> > -                                       req_msg->number_of_ch, req_msg->bit_rate,
> > -                                       datainterval);
> > -
> > -             if (!ret)
> > -                     ret = prepare_qmi_response(subs, req_msg, &resp,
> > -                                                info_idx);
> > -             if (ret < 0) {
> > -                     guard(mutex)(&chip->mutex);
> > -                     subs->opened = 0;
> > -             }
> > -     } else {
> > -             info = &uadev[pcm_card_num].info[info_idx];
> > -             if (info->data_ep_pipe) {
> > -                     ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
> > -                                            info->data_ep_pipe);
> > -                     if (ep) {
> > -                             xhci_sideband_stop_endpoint(uadev[pcm_card_num].sb,
> > -                                                         ep);
> > -                             xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb,
> > -                                                           ep);
> > +     udev = subs->dev;
> > +     scoped_guard(device, &udev->dev) {
> > +             if (req_msg->enable) {
> > +                     ret = enable_audio_stream(subs,
> > +                                             map_pcm_format(req_msg->audio_format),
> > +                                             req_msg->number_of_ch, req_msg->bit_rate,
> > +                                             datainterval);
> > +
> > +                     if (!ret)
> > +                             ret = prepare_qmi_response(subs, req_msg, &resp,
> > +                                                     info_idx);
> > +                     if (ret < 0) {
> > +                             guard(mutex)(&chip->mutex);
> > +                             subs->opened = 0;
> > +                     }
> > +             } else {
> > +                     info = &uadev[pcm_card_num].info[info_idx];
> > +                     if (info->data_ep_pipe) {
> > +                             ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
> > +                                                     info->data_ep_pipe);
> > +                             if (ep) {
> > +                                     xhci_sideband_stop_endpoint(uadev[pcm_card_num].sb,
> > +                                                                     ep);
> > +                                     xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb,
> > +                                                                     ep);
> > +                             }
> > +
> > +                             info->data_ep_pipe = 0;
> >                       }
> >
> > -                     info->data_ep_pipe = 0;
> > -             }
> > -
> > -             if (info->sync_ep_pipe) {
> > -                     ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
> > -                                            info->sync_ep_pipe);
> > -                     if (ep) {
> > -                             xhci_sideband_stop_endpoint(uadev[pcm_card_num].sb,
> > -                                                         ep);
> > -                             xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb,
> > -                                                           ep);
> > +                     if (info->sync_ep_pipe) {
> > +                             ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
> > +                                                     info->sync_ep_pipe);
> > +                             if (ep) {
> > +                                     xhci_sideband_stop_endpoint(uadev[pcm_card_num].sb,
> > +                                                                     ep);
> > +                                     xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb,
> > +                                                                     ep);
> > +                             }
> > +
> > +                             info->sync_ep_pipe = 0;
> >                       }
> >
> > -                     info->sync_ep_pipe = 0;
> > +                     disable_audio_stream(subs);
> > +                     guard(mutex)(&chip->mutex);
> > +                     subs->opened = 0;
> >               }
> > -
> > -             disable_audio_stream(subs);
> > -             guard(mutex)(&chip->mutex);
> > -             subs->opened = 0;
> >       }
> >
> >  response:
> > --
> > 2.53.0.414.gf7e9f6c205-goog
> >