[RFC PATCH] usb: gadget: uvc: Prevent smmu fault in unstopped stream uvc teardown

[RFC PATCH] usb: gadget: uvc: Prevent smmu fault in unstopped stream uvc teardown

[Udipto Goswami]

When switching USB compositions while the camera is streaming, an SMMU
fault can occur because dwc3_remove_requests() unmaps buffers via
dwc3_gadget_giveback() while the controller hardware is still performing
DMA operations on subsequent requests in the started_list.

Fix this by adding a delay in uvc_video_complete() when handling the first
-ESHUTDOWN event  (detected by checking !UVC_QUEUE_DISCONNECTED) to allow
the controller to  complete in-flight DMA and drain its FIFO before
dwc3_remove_requests()  proceeds to unmap remaining buffers,
preventing the SMMU translation fault.

Signed-off-by: Udipto Goswami <udipto.goswami@oss.qualcomm.com>
---
 drivers/usb/gadget/function/uvc_video.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index fb77b0b21790..04724bd44ab9 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -366,7 +366,15 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
 
 	case -ESHUTDOWN:	/* disconnect from host. */
 		uvcg_dbg(&video->uvc->func, "VS request cancelled.\n");
+		if (!(queue->flags & UVC_QUEUE_DISCONNECTED))
+			delay = 1;
 		uvcg_queue_cancel(queue, 1);
+		if (delay) {
+			if (in_interrupt() || irqs_disabled() || in_atomic())
+				 mdelay(1);
+			else
+				msleep(50);
+		}
 		break;
 
 	default:
-- 
2.34.1

Re: [RFC PATCH] usb: gadget: uvc: Prevent smmu fault in unstopped stream uvc teardown

[Udipto Goswami]

On Mon, Jan 5, 2026 at 10:37 AM Udipto Goswami
<udipto.goswami@oss.qualcomm.com> wrote:
>
> When switching USB compositions while the camera is streaming, an SMMU
> fault can occur because dwc3_remove_requests() unmaps buffers via
> dwc3_gadget_giveback() while the controller hardware is still performing
> DMA operations on subsequent requests in the started_list.
>
> Fix this by adding a delay in uvc_video_complete() when handling the first
> -ESHUTDOWN event  (detected by checking !UVC_QUEUE_DISCONNECTED) to allow
> the controller to  complete in-flight DMA and drain its FIFO before
> dwc3_remove_requests()  proceeds to unmap remaining buffers,
> preventing the SMMU translation fault.
>
> Signed-off-by: Udipto Goswami <udipto.goswami@oss.qualcomm.com>
> ---
>  drivers/usb/gadget/function/uvc_video.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
> index fb77b0b21790..04724bd44ab9 100644
> --- a/drivers/usb/gadget/function/uvc_video.c
> +++ b/drivers/usb/gadget/function/uvc_video.c
> @@ -366,7 +366,15 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
>
>         case -ESHUTDOWN:        /* disconnect from host. */
>                 uvcg_dbg(&video->uvc->func, "VS request cancelled.\n");
> +               if (!(queue->flags & UVC_QUEUE_DISCONNECTED))
> +                       delay = 1;

Apologies for the oversight. This revision has a compilation failure
due to a missing declaration of the delay variable.
If the code logic looks sound, I will prepare and send a corrected
version after the review.

>                 uvcg_queue_cancel(queue, 1);
> +               if (delay) {
> +                       if (in_interrupt() || irqs_disabled() || in_atomic())
> +                                mdelay(1);
> +                       else
> +                               msleep(50);
> +               }
>                 break;
>
>         default:
> --
> 2.34.1
>

Re: [RFC PATCH] usb: gadget: uvc: Prevent smmu fault in unstopped stream uvc teardown

[Selvarasu Ganesan]

On 1/5/2026 2:21 PM, Udipto Goswami wrote:
> On Mon, Jan 5, 2026 at 10:37 AM Udipto Goswami
> <udipto.goswami@oss.qualcomm.com> wrote:
>> When switching USB compositions while the camera is streaming, an SMMU
>> fault can occur because dwc3_remove_requests() unmaps buffers via
>> dwc3_gadget_giveback() while the controller hardware is still performing
>> DMA operations on subsequent requests in the started_list.
This may be a problem with how the DWC3 gadget driver handles 
isochronous endpoints when the function driver issues an EP‑disable 
request as part of compositions switch from UVC.

In the current dwc3_ep_disable implementation, the 
dwc3_gadget_giveback() request is issued without waiting for the 
completion of the End‑Transfer command that stops the active transfer.

>>
>> Fix this by adding a delay in uvc_video_complete() when handling the first
>> -ESHUTDOWN event  (detected by checking !UVC_QUEUE_DISCONNECTED) to allow
>> the controller to  complete in-flight DMA and drain its FIFO before
>> dwc3_remove_requests()  proceeds to unmap remaining buffers,
>> preventing the SMMU translation fault.
>>
>> Signed-off-by: Udipto Goswami <udipto.goswami@oss.qualcomm.com>
>> ---
>>   drivers/usb/gadget/function/uvc_video.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
>> index fb77b0b21790..04724bd44ab9 100644
>> --- a/drivers/usb/gadget/function/uvc_video.c
>> +++ b/drivers/usb/gadget/function/uvc_video.c
>> @@ -366,7 +366,15 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
>>
>>          case -ESHUTDOWN:        /* disconnect from host. */
>>                  uvcg_dbg(&video->uvc->func, "VS request cancelled.\n");
>> +               if (!(queue->flags & UVC_QUEUE_DISCONNECTED))
>> +                       delay = 1;
> Apologies for the oversight. This revision has a compilation failure
> due to a missing declaration of the delay variable.
> If the code logic looks sound, I will prepare and send a corrected
> version after the review.
>
>>                  uvcg_queue_cancel(queue, 1);
>> +               if (delay) {
>> +                       if (in_interrupt() || irqs_disabled() || in_atomic())
>> +                                mdelay(1);
>> +                       else
>> +                               msleep(50);
>> +               }
>>                  break;


How you decided this delay time? And any reason why this delay is added 
after uvcg_queue_cancel?

>>
>>          default:
>> --
>> 2.34.1
>>
>

Re: [RFC PATCH] usb: gadget: uvc: Prevent smmu fault in unstopped stream uvc teardown

[Udipto Goswami]

On Mon, Jan 5, 2026 at 3:15 PM Selvarasu Ganesan
<selvarasu.g@samsung.com> wrote:
>
>
> On 1/5/2026 2:21 PM, Udipto Goswami wrote:
> > On Mon, Jan 5, 2026 at 10:37 AM Udipto Goswami
> > <udipto.goswami@oss.qualcomm.com> wrote:
> >> When switching USB compositions while the camera is streaming, an SMMU
> >> fault can occur because dwc3_remove_requests() unmaps buffers via
> >> dwc3_gadget_giveback() while the controller hardware is still performing
> >> DMA operations on subsequent requests in the started_list.
> This may be a problem with how the DWC3 gadget driver handles
> isochronous endpoints when the function driver issues an EP‑disable
> request as part of compositions switch from UVC.
>
> In the current dwc3_ep_disable implementation, the
> dwc3_gadget_giveback() request is issued without waiting for the
> completion of the End‑Transfer command that stops the active transfer.

Yes Selva,
not sure if polling for CMDACT is a good option in the controller though.
What i understand the IOC for isoc eps is not set so we won't know
when the end transfer has completed, please correct me if i'm wrong
here.
>
> >>
> >> Fix this by adding a delay in uvc_video_complete() when handling the first
> >> -ESHUTDOWN event  (detected by checking !UVC_QUEUE_DISCONNECTED) to allow
> >> the controller to  complete in-flight DMA and drain its FIFO before
> >> dwc3_remove_requests()  proceeds to unmap remaining buffers,
> >> preventing the SMMU translation fault.
> >>
> >> Signed-off-by: Udipto Goswami <udipto.goswami@oss.qualcomm.com>
> >> ---
> >>   drivers/usb/gadget/function/uvc_video.c | 8 ++++++++
> >>   1 file changed, 8 insertions(+)
> >>
> >> diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
> >> index fb77b0b21790..04724bd44ab9 100644
> >> --- a/drivers/usb/gadget/function/uvc_video.c
> >> +++ b/drivers/usb/gadget/function/uvc_video.c
> >> @@ -366,7 +366,15 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
> >>
> >>          case -ESHUTDOWN:        /* disconnect from host. */
> >>                  uvcg_dbg(&video->uvc->func, "VS request cancelled.\n");
> >> +               if (!(queue->flags & UVC_QUEUE_DISCONNECTED))
> >> +                       delay = 1;
> > Apologies for the oversight. This revision has a compilation failure
> > due to a missing declaration of the delay variable.
> > If the code logic looks sound, I will prepare and send a corrected
> > version after the review.
> >
> >>                  uvcg_queue_cancel(queue, 1);
> >> +               if (delay) {
> >> +                       if (in_interrupt() || irqs_disabled() || in_atomic())
> >> +                                mdelay(1);
> >> +                       else
> >> +                               msleep(50);
> >> +               }
> >>                  break;
>
>
> How you decided this delay time? And any reason why this delay is added
> after uvcg_queue_cancel?

this was based on a few hit and trails we tried from 10ms and reached to 50ms.
We 50ms was an ideal case where  the trbs had enough time to complete
accessing the DMA and release i believe.
I understand that the delay time has no basis however could be a concern.

From what I understand, uvcg_queue_cancel()  will remove the request
from the video->queue and the UVC_QUEUE_DISCONNECTED will be set.
So we want the active requests to be removed before doing so.

Thanks,
-Udipto

Re: [RFC PATCH] usb: gadget: uvc: Prevent smmu fault in unstopped stream uvc teardown

[Alan Stern]

On Mon, Jan 05, 2026 at 10:37:24AM +0530, Udipto Goswami wrote:
> When switching USB compositions while the camera is streaming, an SMMU
> fault can occur because dwc3_remove_requests() unmaps buffers via
> dwc3_gadget_giveback() while the controller hardware is still performing
> DMA operations on subsequent requests in the started_list.
> 
> Fix this by adding a delay in uvc_video_complete() when handling the first
> -ESHUTDOWN event  (detected by checking !UVC_QUEUE_DISCONNECTED) to allow
> the controller to  complete in-flight DMA and drain its FIFO before
> dwc3_remove_requests()  proceeds to unmap remaining buffers,
> preventing the SMMU translation fault.

Wouldn't it be better to wait for the in-flight URBs to complete, so 
that you _know_ the controller is finished with them, instead of 
delaying for some arbitrary and possibly too-short amount of time?

Also, wouldn't it be better to avoid calling mdelay() while in interrupt 
or atomic context?

Alan Stern

> Signed-off-by: Udipto Goswami <udipto.goswami@oss.qualcomm.com>
> ---
>  drivers/usb/gadget/function/uvc_video.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
> index fb77b0b21790..04724bd44ab9 100644
> --- a/drivers/usb/gadget/function/uvc_video.c
> +++ b/drivers/usb/gadget/function/uvc_video.c
> @@ -366,7 +366,15 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
>  
>  	case -ESHUTDOWN:	/* disconnect from host. */
>  		uvcg_dbg(&video->uvc->func, "VS request cancelled.\n");
> +		if (!(queue->flags & UVC_QUEUE_DISCONNECTED))
> +			delay = 1;
>  		uvcg_queue_cancel(queue, 1);
> +		if (delay) {
> +			if (in_interrupt() || irqs_disabled() || in_atomic())
> +				 mdelay(1);
> +			else
> +				msleep(50);
> +		}
>  		break;
>  
>  	default:

Re: [RFC PATCH] usb: gadget: uvc: Prevent smmu fault in unstopped stream uvc teardown

[Udipto Goswami]

On Mon, Jan 5, 2026 at 9:25 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Mon, Jan 05, 2026 at 10:37:24AM +0530, Udipto Goswami wrote:
> > When switching USB compositions while the camera is streaming, an SMMU
> > fault can occur because dwc3_remove_requests() unmaps buffers via
> > dwc3_gadget_giveback() while the controller hardware is still performing
> > DMA operations on subsequent requests in the started_list.
> >
> > Fix this by adding a delay in uvc_video_complete() when handling the first
> > -ESHUTDOWN event  (detected by checking !UVC_QUEUE_DISCONNECTED) to allow
> > the controller to  complete in-flight DMA and drain its FIFO before
> > dwc3_remove_requests()  proceeds to unmap remaining buffers,
> > preventing the SMMU translation fault.
>
> Wouldn't it be better to wait for the in-flight URBs to complete, so
> that you _know_ the controller is finished with them, instead of
> delaying for some arbitrary and possibly too-short amount of time?
>
> Also, wouldn't it be better to avoid calling mdelay() while in interrupt
> or atomic context?
>
> Alan Stern

Hi Alan,
True, but i think we wouldn't know for ISOC eps whether the TRB got
completed or not since CMDIOC isn't set for these,
We also tried to address the same from the gadget.c dwc3_send_gadget_ep_cmd()
Something like this:

- if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_ENDTRANSFER &&

-   !(cmd & DWC3_DEPCMD_CMDIOC))
- mdelay(1);
+ if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_ENDTRANSFER &&
+   !(cmd & DWC3_DEPCMD_CMDIOC)){
+   if(usb_endpoint_xfer_isoc(desc))
+ mdelay(10);
+ else
+ mdelay(1);
+ }
But this didn't help mitigate it.

mdelay is busy wait (without sleep) so I thought it should be fine,
but yah if you see any risk with this I can try removing these.
Moreover our problem here is no one from the userspace or the upper
layer called a streamoff which is why the requests are still in
flight.
We do not see this issue if a disconnect had been performed while in
flight, it seems there is a different approach which disconnect takes
to handle this.

Thanks,
-Udipto

Re: [RFC PATCH] usb: gadget: uvc: Prevent smmu fault in unstopped stream uvc teardown

[Alan Stern]

On Tue, Jan 06, 2026 at 04:13:21PM +0530, Udipto Goswami wrote:
> On Mon, Jan 5, 2026 at 9:25 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> > On Mon, Jan 05, 2026 at 10:37:24AM +0530, Udipto Goswami wrote:
> > > When switching USB compositions while the camera is streaming, an SMMU
> > > fault can occur because dwc3_remove_requests() unmaps buffers via
> > > dwc3_gadget_giveback() while the controller hardware is still performing
> > > DMA operations on subsequent requests in the started_list.
> > >
> > > Fix this by adding a delay in uvc_video_complete() when handling the first
> > > -ESHUTDOWN event  (detected by checking !UVC_QUEUE_DISCONNECTED) to allow
> > > the controller to  complete in-flight DMA and drain its FIFO before
> > > dwc3_remove_requests()  proceeds to unmap remaining buffers,
> > > preventing the SMMU translation fault.
> >
> > Wouldn't it be better to wait for the in-flight URBs to complete, so
> > that you _know_ the controller is finished with them, instead of
> > delaying for some arbitrary and possibly too-short amount of time?
> >
> > Also, wouldn't it be better to avoid calling mdelay() while in interrupt
> > or atomic context?
> >
> > Alan Stern
> 
> Hi Alan,
> True, but i think we wouldn't know for ISOC eps whether the TRB got
> completed or not since CMDIOC isn't set for these,

Look, you know a lot more about what's going on here than I do.  
Describing the details to me won't help because I hardly understand 
anything about how these drivers work to begin with.  I'm just trying to 
prevent you from applying a bad fix.

Start by asking where the real problem begins.  Is the problem caused by 
the uvc driver skipping a step?  Or is there a bug in dwc3?  Either way, 
the problem should be cured at its source, not by adding some arbitrary 
delay later on.

> We also tried to address the same from the gadget.c dwc3_send_gadget_ep_cmd()
> Something like this:
> 
> - if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_ENDTRANSFER &&
> 
> -   !(cmd & DWC3_DEPCMD_CMDIOC))
> - mdelay(1);
> + if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_ENDTRANSFER &&
> +   !(cmd & DWC3_DEPCMD_CMDIOC)){
> +   if(usb_endpoint_xfer_isoc(desc))
> + mdelay(10);
> + else
> + mdelay(1);
> + }
> But this didn't help mitigate it.

Again, the details mean nothing to me.  But delays are always 
suspicious, especially when there's no indication of how the delay's 
length was chosen.

> mdelay is busy wait (without sleep) so I thought it should be fine,
> but yah if you see any risk with this I can try removing these.

It's okay in the sense that mdelay in interrupt or atomic context won't 
crash the kernel (whereas msleep() would), but it's still a bad idea 
unless it's totally unavoidable.  A delay causes the kernel to waste 
time when it could be busy doing something else -- not a good thing to 
do.  And if you really have no other choice, you should make sure that 
the delay is as short as possible -- say, no more than one or two 
milliseconds.  A 10-ms delay is pretty much unacceptable.

> Moreover our problem here is no one from the userspace or the upper
> layer called a streamoff which is why the requests are still in
> flight.

Then maybe the upper layer should be changed so that it does call 
streamoff?  While I'm not familiar with any of the details, it seems 
clear that an upper layer should tell a lower layer when it has finished 
using some resource that the lower layer provides.

> We do not see this issue if a disconnect had been performed while in
> flight, it seems there is a different approach which disconnect takes
> to handle this.

Probably a disconnect causes the transfer to be cancelled immediately 
rather than waiting around for it to finish by itself.

Alan Stern

Re: [RFC PATCH] usb: gadget: uvc: Prevent smmu fault in unstopped stream uvc teardown

[Udipto Goswami]

On Tue, Jan 6, 2026 at 10:23 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Tue, Jan 06, 2026 at 04:13:21PM +0530, Udipto Goswami wrote:
> > On Mon, Jan 5, 2026 at 9:25 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> > >
> > > On Mon, Jan 05, 2026 at 10:37:24AM +0530, Udipto Goswami wrote:
> > > > When switching USB compositions while the camera is streaming, an SMMU
> > > > fault can occur because dwc3_remove_requests() unmaps buffers via
> > > > dwc3_gadget_giveback() while the controller hardware is still performing
> > > > DMA operations on subsequent requests in the started_list.
> > > >
> > > > Fix this by adding a delay in uvc_video_complete() when handling the first
> > > > -ESHUTDOWN event  (detected by checking !UVC_QUEUE_DISCONNECTED) to allow
> > > > the controller to  complete in-flight DMA and drain its FIFO before
> > > > dwc3_remove_requests()  proceeds to unmap remaining buffers,
> > > > preventing the SMMU translation fault.
> > >
> > > Wouldn't it be better to wait for the in-flight URBs to complete, so
> > > that you _know_ the controller is finished with them, instead of
> > > delaying for some arbitrary and possibly too-short amount of time?
> > >
> > > Also, wouldn't it be better to avoid calling mdelay() while in interrupt
> > > or atomic context?
> > >
> > > Alan Stern
> >
> > Hi Alan,
> > True, but i think we wouldn't know for ISOC eps whether the TRB got
> > completed or not since CMDIOC isn't set for these,
>
> Look, you know a lot more about what's going on here than I do.
> Describing the details to me won't help because I hardly understand
> anything about how these drivers work to begin with.  I'm just trying to
> prevent you from applying a bad fix.
Hi Alan,
I understand, thanks for the review.
>
> Start by asking where the real problem begins.  Is the problem caused by
> the uvc driver skipping a step?  Or is there a bug in dwc3?  Either way,
> the problem should be cured at its source, not by adding some arbitrary
> delay later on.
I agree with you,
we started seeing this  when we tried switching for uvc,adb to mtp,adb
when stream off not done.
This is the starting point of our failure.


>
> > We also tried to address the same from the gadget.c dwc3_send_gadget_ep_cmd()
> > Something like this:
> >
> > - if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_ENDTRANSFER &&
> >
> > -   !(cmd & DWC3_DEPCMD_CMDIOC))
> > - mdelay(1);
> > + if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_ENDTRANSFER &&
> > +   !(cmd & DWC3_DEPCMD_CMDIOC)){
> > +   if(usb_endpoint_xfer_isoc(desc))
> > + mdelay(10);
> > + else
> > + mdelay(1);
> > + }
> > But this didn't help mitigate it.
>
> Again, the details mean nothing to me.  But delays are always
> suspicious, especially when there's no indication of how the delay's
> length was chosen.
got it.
>
> > mdelay is busy wait (without sleep) so I thought it should be fine,
> > but yah if you see any risk with this I can try removing these.
>
> It's okay in the sense that mdelay in interrupt or atomic context won't
> crash the kernel (whereas msleep() would), but it's still a bad idea
> unless it's totally unavoidable.  A delay causes the kernel to waste
> time when it could be busy doing something else -- not a good thing to
> do.  And if you really have no other choice, you should make sure that
> the delay is as short as possible -- say, no more than one or two
> milliseconds.  A 10-ms delay is pretty much unacceptable.
understood.
>
> > Moreover our problem here is no one from the userspace or the upper
> > layer called a streamoff which is why the requests are still in
> > flight.
>
> Then maybe the upper layer should be changed so that it does call
> streamoff?  While I'm not familiar with any of the details, it seems
> clear that an upper layer should tell a lower layer when it has finished
> using some resource that the lower layer provides.

Yah I think I can try to explore this option where we force streamoff.
>
> > We do not see this issue if a disconnect had been performed while in
> > flight, it seems there is a different approach which disconnect takes
> > to handle this.
>
> Probably a disconnect causes the transfer to be cancelled immediately
> rather than waiting around for it to finish by itself.
let me get these details on what happens when composition switches and
disconnects.
Thanks for the suggestion above, let me try that out with streamoff.

Thanks,
-Udipto

Re: [RFC PATCH] usb: gadget: uvc: Prevent smmu fault in unstopped stream uvc teardown

[Thinh Nguyen]

On Wed, Jan 07, 2026, Udipto Goswami wrote:
> On Tue, Jan 6, 2026 at 10:23 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> > Probably a disconnect causes the transfer to be cancelled immediately
> > rather than waiting around for it to finish by itself.

> let me get these details on what happens when composition switches and
> disconnects.
> Thanks for the suggestion above, let me try that out with streamoff.
> 
> Thanks,
> -Udipto
> 

This is probably related to this conversation:

https://lore.kernel.org/linux-usb/20251121022156.vbnheb6r2ytov7bt@synopsys.com/

The summary:
usb_ep_disable() is documented and expected to be in used interrupt
context, and the composite framework and some gadget drivers treat it
so. The dwc3 driver does not "wait" for the endpoints to be flushed
before giving back the usb requests.

The proper fix:
Change usb_ep_disable documentation back to allow to be used in process
context. Update the dwc3 to wait for the endpoint to be flushed before
completing usb_ep_disable. Audit the composite framework and gadget
drivers to not use usb_ep_disable in interrupt context.

BR,
Thinh