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@xxxxxxx> 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@xxxxxxxxxxxxxxx
> > Fixes: ef82a4803aab ("xhci: sideband: add api to trace sideband usage")
> > Signed-off-by: Guan-Yu Lin <guanyulin@xxxxxxxxxx>
>
> 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
> >