[PATCH v6 3/4] ALSA: usb-audio: qcom: tag sideband endpoints before ring allocation

[Wesley Cheng]

On kernels with a PAGE_SIZE larger than TRB_SEGMENT_SIZE,
xhci_endpoint_init() allocates the ring segments of endpoints that have
already been tagged via xhci_sideband_add_endpoint() with
dma_alloc_coherent() instead of from the shared xhci->segment_pool, so
the buffer reported to the ADSP over QMI is guaranteed to come from a
page of its own.

xhci_sideband_add_endpoint() must therefore run before the endpoint's
transfer ring is first allocated (i.e. before snd_usb_endpoint_prepare()
triggers xhci_endpoint_init()) for that allocation to apply. Move the
xhci_sideband_add_endpoint() calls out of uaudio_endpoint_setup() and
into enable_audio_stream(), before snd_usb_endpoint_prepare() is called
for the data and sync endpoints, and unwind them on the new error paths.

Now that its only caller tags the endpoint before the ring exists, have
xhci_sideband_add_endpoint() return -EBUSY if the endpoint already has a
transfer ring, so a sideband client that adds the endpoint too late gets
an error instead of a ring that silently came from the shared pool.

At that point in the setup sequence dev->ep_in[]/ep_out[] are not yet
populated, since the endpoint's altsetting has not been activated, so
usb_pipe_endpoint() cannot be used to find the usb_host_endpoint. Add
uaudio_find_host_endpoint(), which resolves it directly from the
interface's altsetting descriptor table instead.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Wesley Cheng <wesley.cheng@xxxxxxxxxxxxxxxx>
---
 drivers/usb/host/xhci-sideband.c  |   4 +-
 sound/usb/qcom/qc_audio_offload.c | 158 ++++++++++++++++++++++++++++----------
 2 files changed, 119 insertions(+), 43 deletions(-)

diff --git a/drivers/usb/host/xhci-sideband.c b/drivers/usb/host/xhci-sideband.c
index a88698b085c7..7c3a2daef5ce 100644
--- a/drivers/usb/host/xhci-sideband.c
+++ b/drivers/usb/host/xhci-sideband.c
@@ -151,6 +151,8 @@ EXPORT_SYMBOL_GPL(xhci_sideband_notify_ep_ring_free);
  * After an endpoint is added the sideband client can get the endpoint transfer
  * ring buffer by calling xhci_sideband_endpoint_buffer()
  *
+ * The endpoint must be added before its transfer ring is allocated.
+ *
  * Return: 0 on success, negative error otherwise.
  */
 int
@@ -179,7 +181,7 @@ xhci_sideband_add_endpoint(struct xhci_sideband *sb,
 	 * and let this function add the endpoint and allocate the ring buffer
 	 * with the smallest common DMA mask
 	 */
-	if (sb->eps[ep_index] || ep->sideband)
+	if (sb->eps[ep_index] || ep->sideband || ep->ring || ep->new_ring)
 		return -EBUSY;
 
 	/*
diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
index e4bfd43a2488..c8edc48cb80e 100644
--- a/sound/usb/qcom/qc_audio_offload.c
+++ b/sound/usb/qcom/qc_audio_offload.c
@@ -941,6 +941,40 @@ static void uaudio_dev_release(struct kref *kref)
 	wake_up(&dev->disconnect_wq);
 }
 
+/**
+ * uaudio_find_host_endpoint() - look up usb_host_endpoint for a snd_usb_endpoint
+ * @subs: usb substream owning the target snd_usb_endpoint
+ * @endpoint: sync or data snd_usb_endpoint to resolve
+ *
+ * usb_pipe_endpoint() resolves via dev->ep_in[]/ep_out[], which are only
+ * populated after usb_set_interface() activates the endpoint's altsetting.
+ * Look the endpoint up directly in that altsetting before preparation instead.
+ *
+ * Return: matching usb_host_endpoint, or NULL if not found.
+ */
+static struct usb_host_endpoint *
+uaudio_find_host_endpoint(struct snd_usb_substream *subs,
+			  struct snd_usb_endpoint *endpoint)
+{
+	struct usb_host_interface *alt;
+	struct usb_interface *iface;
+	int i;
+
+	iface = usb_ifnum_to_if(subs->dev, endpoint->iface);
+	if (!iface)
+		return NULL;
+
+	alt = usb_altnum_to_altsetting(iface, endpoint->altsetting);
+	if (!alt)
+		return NULL;
+
+	for (i = 0; i < alt->desc.bNumEndpoints; i++)
+		if (alt->endpoint[i].desc.bEndpointAddress == endpoint->ep_num)
+			return &alt->endpoint[i];
+
+	return NULL;
+}
+
 /**
  * enable_audio_stream() - enable usb snd endpoints
  * @subs: usb substream
@@ -948,6 +982,7 @@ static void uaudio_dev_release(struct kref *kref)
  * @channels: number of channels
  * @cur_rate: sample rate
  * @datainterval: interval
+ * @card_num: uadev index
  *
  * Opens all USB SND endpoints used for the data interface.  This will increment
  * the USB SND endpoint's opened count.  Requests to keep the interface resumed
@@ -958,8 +993,9 @@ static void uaudio_dev_release(struct kref *kref)
 static int enable_audio_stream(struct snd_usb_substream *subs,
 			       snd_pcm_format_t pcm_format,
 			       unsigned int channels, unsigned int cur_rate,
-			       int datainterval)
+			       int datainterval, unsigned int card_num)
 {
+	struct usb_host_endpoint *data_ep = NULL, *sync_ep = NULL;
 	struct snd_pcm_hw_params params;
 	struct snd_usb_audio *chip;
 	struct snd_interval *i;
@@ -987,42 +1023,75 @@ static int enable_audio_stream(struct snd_usb_substream *subs,
 	snd_usb_autoresume(chip);
 
 	ret = snd_usb_hw_params(subs, &params);
-	if (ret < 0)
-		goto put_suspend;
+	if (ret < 0) {
+		snd_usb_autosuspend(chip);
+		return ret;
+	}
+
+	CLASS(snd_usb_lock, pm)(chip);
+	if (pm.err < 0) {
+		ret = pm.err;
+		goto detach_ep;
+	}
+
+	data_ep = uaudio_find_host_endpoint(subs, subs->data_endpoint);
+	if (!data_ep) {
+		dev_err(&subs->dev->dev, "data ep # %d not found\n",
+			subs->data_endpoint->ep_num);
+		ret = -ENODEV;
+		goto detach_ep;
+	}
+
+	ret = xhci_sideband_add_endpoint(uadev[card_num].sb, data_ep);
+	if (ret < 0) {
+		dev_err(&subs->dev->dev,
+			"failed to add data ep to sideband: %d\n", ret);
+		goto detach_ep;
+	}
 
-	if (!atomic_read(&chip->shutdown)) {
-		CLASS(snd_usb_lock, pm)(chip);
-		if (pm.err < 0) {
-			ret = pm.err;
-			goto detach_ep;
+	if (subs->sync_endpoint) {
+		sync_ep = uaudio_find_host_endpoint(subs, subs->sync_endpoint);
+		if (!sync_ep) {
+			dev_err(&subs->dev->dev, "sync ep # %d not found\n",
+				subs->sync_endpoint->ep_num);
+			ret = -ENODEV;
+			goto remove_data_ep;
 		}
 
-		if (subs->sync_endpoint) {
-			ret = snd_usb_endpoint_prepare(chip, subs->sync_endpoint);
-			if (ret < 0)
-				goto detach_ep;
+		ret = xhci_sideband_add_endpoint(uadev[card_num].sb, sync_ep);
+		if (ret < 0) {
+			dev_err(&subs->dev->dev,
+				"failed to add sync ep to sideband: %d\n", ret);
+			goto remove_data_ep;
 		}
 
-		ret = snd_usb_endpoint_prepare(chip, subs->data_endpoint);
+		ret = snd_usb_endpoint_prepare(chip, subs->sync_endpoint);
 		if (ret < 0)
-			goto detach_ep;
-
-		dev_dbg(uaudio_qdev->data->dev,
-			"selected %s iface:%d altsetting:%d datainterval:%dus\n",
-			subs->direction ? "capture" : "playback",
-			subs->cur_audiofmt->iface, subs->cur_audiofmt->altsetting,
-			(1 << subs->cur_audiofmt->datainterval) *
-			(subs->dev->speed >= USB_SPEED_HIGH ?
-			BUS_INTERVAL_HIGHSPEED_AND_ABOVE :
-			BUS_INTERVAL_FULL_SPEED));
+			goto remove_sync_ep;
 	}
 
+	ret = snd_usb_endpoint_prepare(chip, subs->data_endpoint);
+	if (ret < 0)
+		goto remove_sync_ep;
+
+	dev_dbg(uaudio_qdev->data->dev,
+		"selected %s iface:%d altsetting:%d datainterval:%dus\n",
+		subs->direction ? "capture" : "playback",
+		subs->cur_audiofmt->iface, subs->cur_audiofmt->altsetting,
+		(1 << subs->cur_audiofmt->datainterval) *
+		(subs->dev->speed >= USB_SPEED_HIGH ?
+		BUS_INTERVAL_HIGHSPEED_AND_ABOVE :
+		BUS_INTERVAL_FULL_SPEED));
+
 	return 0;
 
+remove_sync_ep:
+	if (sync_ep)
+		xhci_sideband_remove_endpoint(uadev[card_num].sb, sync_ep);
+remove_data_ep:
+	xhci_sideband_remove_endpoint(uadev[card_num].sb, data_ep);
 detach_ep:
 	snd_usb_hw_free(subs);
-
-put_suspend:
 	snd_usb_autosuspend(chip);
 
 	return ret;
@@ -1140,20 +1209,12 @@ uaudio_endpoint_setup(struct snd_usb_substream *subs,
 
 	memcpy(ep_desc, &ep->desc, sizeof(ep->desc));
 
-	ret = xhci_sideband_add_endpoint(uadev[card_num].sb, ep);
-	if (ret < 0) {
-		dev_err(&subs->dev->dev,
-			"failed to add data ep to sec intr: %d\n", ret);
-		ret = -ENODEV;
-		goto exit;
-	}
-
 	sgt = xhci_sideband_get_endpoint_buffer(uadev[card_num].sb, ep);
 	if (!sgt) {
 		dev_err(&subs->dev->dev,
 			"failed to get data ep ring address: %d\n", ret);
 		ret = -ENODEV;
-		goto remove_ep;
+		goto exit;
 	}
 
 	pg = sg_page(sgt->sgl);
@@ -1177,8 +1238,6 @@ uaudio_endpoint_setup(struct snd_usb_substream *subs,
 
 clear_pa:
 	mem_info->dma = 0;
-remove_ep:
-	xhci_sideband_remove_endpoint(uadev[card_num].sb, ep);
 exit:
 	return ret;
 }
@@ -1519,14 +1578,10 @@ static int prepare_qmi_response(struct snd_usb_substream *subs,
 		uaudio_iommu_unmap(MEM_XFER_RING,
 				   IOVA_MASK(resp->xhci_mem_info.tr_sync.iova),
 				   PAGE_SIZE, PAGE_SIZE);
-		xhci_sideband_remove_endpoint(uadev[card_num].sb,
-			usb_pipe_endpoint(subs->dev, subs->sync_endpoint->pipe));
 	}
 drop_data_ep:
 	uaudio_iommu_unmap(MEM_XFER_RING, IOVA_MASK(resp->xhci_mem_info.tr_data.iova),
 			   PAGE_SIZE, PAGE_SIZE);
-	xhci_sideband_remove_endpoint(uadev[card_num].sb,
-			usb_pipe_endpoint(subs->dev, subs->data_endpoint->pipe));
 
 	return ret;
 }
@@ -1637,11 +1692,30 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
 		ret = enable_audio_stream(subs,
 					  map_pcm_format(req_msg->audio_format),
 					  req_msg->number_of_ch, req_msg->bit_rate,
-					  datainterval);
+					  datainterval, pcm_card_num);
 
-		if (!ret)
+		if (!ret) {
 			ret = prepare_qmi_response(subs, req_msg, &resp,
 						   info_idx);
+			if (ret < 0) {
+				struct xhci_sideband *sb = uadev[pcm_card_num].sb;
+
+				if (subs->sync_endpoint) {
+					ep = usb_pipe_endpoint(subs->dev,
+							       subs->sync_endpoint->pipe);
+					if (ep)
+						xhci_sideband_remove_endpoint(sb, ep);
+				}
+
+				ep = usb_pipe_endpoint(subs->dev,
+						       subs->data_endpoint->pipe);
+				if (ep)
+					xhci_sideband_remove_endpoint(sb, ep);
+
+				disable_audio_stream(subs);
+			}
+		}
+
 		if (ret < 0) {
 			guard(mutex)(&chip->mutex);
 			subs->opened = 0;

-- 
2.43.0