linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop
@ 2021-08-27  7:58 Jerome Brunet
  2021-08-27  8:05 ` Felipe Balbi
  0 siblings, 1 reply; 11+ messages in thread
From: Jerome Brunet @ 2021-08-27  7:58 UTC (permalink / raw)
  To: Ruslan Bilovol, Felipe Balbi, Pavel Hofman
  Cc: Jerome Brunet, Greg Kroah-Hartman, Jack Pham, linux-kernel,
	linux-usb, Thinh Nguyen, Ferry Toth

When the uac2 function is stopped, there seems to be an issue reported on
some platforms (Intel Merrifield at least)

BUG: kernel NULL pointer dereference, address: 0000000000000008
...
RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
...
Call Trace:
 dwc3_remove_requests.constprop.0+0x12f/0x170
 __dwc3_gadget_ep_disable+0x7a/0x160
 dwc3_gadget_ep_disable+0x3d/0xd0
 usb_ep_disable+0x1c/0x70
 u_audio_stop_capture+0x79/0x120 [u_audio]
 afunc_set_alt+0x73/0x80 [usb_f_uac2]
 composite_setup+0x224/0x1b90 [libcomposite]

The issue happens only when the gadget is using the sync type "async", not
"adaptive". This indicates that problem is coming from the feedback
endpoint, which is only used with async synchronization mode.

The problem is that request is freed regardless of usb_ep_dequeue(), which
ends up badly if the request is not actually dequeued yet.

Update the feedback endpoint free function to release the endpoint the same
way it is done for the data endpoint, which takes care of the problem.

Reported-by: Ferry Toth <ftoth@exalondelft.nl>
Tested-by: Ferry Toth <ftoth@exalondelft.nl>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 Hi Felipe,

 This solves the issue reported here [0] and makes revert [1]
 unnecessary.

 [0]: https://lore.kernel.org/r/20210824201433.11385-1-ftoth@exalondelft.nl
 [1]: https://lore.kernel.org/r/20210826185739.3868-1-ftoth@exalondelft.nl

 drivers/usb/gadget/function/u_audio.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index 018dd0978995..63d9340f008e 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -230,7 +230,13 @@ static void u_audio_iso_fback_complete(struct usb_ep *ep,
 	int status = req->status;
 
 	/* i/f shutting down */
-	if (!prm->fb_ep_enabled || req->status == -ESHUTDOWN)
+	if (!prm->fb_ep_enabled) {
+		kfree(req->buf);
+		usb_ep_free_request(ep, req);
+		return;
+	}
+
+	if (req->status == -ESHUTDOWN)
 		return;
 
 	/*
@@ -421,9 +427,10 @@ static inline void free_ep_fback(struct uac_rtd_params *prm, struct usb_ep *ep)
 	prm->fb_ep_enabled = false;
 
 	if (prm->req_fback) {
-		usb_ep_dequeue(ep, prm->req_fback);
-		kfree(prm->req_fback->buf);
-		usb_ep_free_request(ep, prm->req_fback);
+		if (usb_ep_dequeue(ep, prm->req_fback)) {
+			kfree(prm->req_fback->buf);
+			usb_ep_free_request(ep, prm->req_fback);
+		}
 		prm->req_fback = NULL;
 	}
 
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop
  2021-08-27  7:58 [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop Jerome Brunet
@ 2021-08-27  8:05 ` Felipe Balbi
  2021-08-27  9:26   ` Jerome Brunet
  0 siblings, 1 reply; 11+ messages in thread
From: Felipe Balbi @ 2021-08-27  8:05 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Ruslan Bilovol, Pavel Hofman, Greg Kroah-Hartman, Jack Pham,
	linux-kernel, linux-usb, Thinh Nguyen, Ferry Toth


Hi,

Jerome Brunet <jbrunet@baylibre.com> writes:

> When the uac2 function is stopped, there seems to be an issue reported on
> some platforms (Intel Merrifield at least)
>
> BUG: kernel NULL pointer dereference, address: 0000000000000008
> ...
> RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
> ...
> Call Trace:
>  dwc3_remove_requests.constprop.0+0x12f/0x170
>  __dwc3_gadget_ep_disable+0x7a/0x160
>  dwc3_gadget_ep_disable+0x3d/0xd0
>  usb_ep_disable+0x1c/0x70
>  u_audio_stop_capture+0x79/0x120 [u_audio]
>  afunc_set_alt+0x73/0x80 [usb_f_uac2]
>  composite_setup+0x224/0x1b90 [libcomposite]
>
> The issue happens only when the gadget is using the sync type "async", not
> "adaptive". This indicates that problem is coming from the feedback
> endpoint, which is only used with async synchronization mode.
>
> The problem is that request is freed regardless of usb_ep_dequeue(), which
> ends up badly if the request is not actually dequeued yet.
>
> Update the feedback endpoint free function to release the endpoint the same
> way it is done for the data endpoint, which takes care of the problem.
>
> Reported-by: Ferry Toth <ftoth@exalondelft.nl>
> Tested-by: Ferry Toth <ftoth@exalondelft.nl>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  Hi Felipe,
>
>  This solves the issue reported here [0] and makes revert [1]
>  unnecessary.

awesome work! Thanks

Acked-by: Felipe Balbi <balbi@kernel.org>

-- 
balbi

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop
  2021-08-27  8:05 ` Felipe Balbi
@ 2021-08-27  9:26   ` Jerome Brunet
  0 siblings, 0 replies; 11+ messages in thread
From: Jerome Brunet @ 2021-08-27  9:26 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Ruslan Bilovol, Pavel Hofman, Greg Kroah-Hartman, Jack Pham,
	linux-kernel, linux-usb, Thinh Nguyen, Ferry Toth


On Fri 27 Aug 2021 at 11:05, Felipe Balbi <balbi@kernel.org> wrote:

> Hi,
>
> Jerome Brunet <jbrunet@baylibre.com> writes:
>
>> When the uac2 function is stopped, there seems to be an issue reported on
>> some platforms (Intel Merrifield at least)
>>
>> BUG: kernel NULL pointer dereference, address: 0000000000000008
>> ...
>> RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
>> ...
>> Call Trace:
>>  dwc3_remove_requests.constprop.0+0x12f/0x170
>>  __dwc3_gadget_ep_disable+0x7a/0x160
>>  dwc3_gadget_ep_disable+0x3d/0xd0
>>  usb_ep_disable+0x1c/0x70
>>  u_audio_stop_capture+0x79/0x120 [u_audio]
>>  afunc_set_alt+0x73/0x80 [usb_f_uac2]
>>  composite_setup+0x224/0x1b90 [libcomposite]
>>
>> The issue happens only when the gadget is using the sync type "async", not
>> "adaptive". This indicates that problem is coming from the feedback
>> endpoint, which is only used with async synchronization mode.
>>
>> The problem is that request is freed regardless of usb_ep_dequeue(), which
>> ends up badly if the request is not actually dequeued yet.
>>
>> Update the feedback endpoint free function to release the endpoint the same
>> way it is done for the data endpoint, which takes care of the problem.
>>
>> Reported-by: Ferry Toth <ftoth@exalondelft.nl>
>> Tested-by: Ferry Toth <ftoth@exalondelft.nl>
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>>  Hi Felipe,
>>
>>  This solves the issue reported here [0] and makes revert [1]
>>  unnecessary.
>
> awesome work! Thanks
>
> Acked-by: Felipe Balbi <balbi@kernel.org>

Forgot the fixes tag

Fixes: 24f779dac8f3 ("usb: gadget: f_uac2/u_audio: add feedback endpoint support")

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop
  2021-08-27  8:38           ` Jerome Brunet
@ 2021-08-27 23:30             ` Thinh Nguyen
  0 siblings, 0 replies; 11+ messages in thread
From: Thinh Nguyen @ 2021-08-27 23:30 UTC (permalink / raw)
  To: Jerome Brunet, Thinh Nguyen, Ferry Toth, Ferry Toth,
	Ruslan Bilovol, Felipe Balbi, Pavel Hofman
  Cc: Greg Kroah-Hartman, Jack Pham, linux-kernel, linux-usb

Jerome Brunet wrote:
> 
> On Fri 27 Aug 2021 at 00:50, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
> 
>>>>>> diff --git a/drivers/usb/gadget/function/u_audio.c
>>>>>> b/drivers/usb/gadget/function/u_audio.c
>>>>>> index 018dd0978995..63d9340f008e 100644
>>>>>> --- a/drivers/usb/gadget/function/u_audio.c
>>>>>> +++ b/drivers/usb/gadget/function/u_audio.c
>>>>>> @@ -230,7 +230,13 @@ static void u_audio_iso_fback_complete(struct
>>>>>> usb_ep *ep,
>>>>>>       int status = req->status;
>>>>>>         /* i/f shutting down */
>>>>>> -    if (!prm->fb_ep_enabled || req->status == -ESHUTDOWN)
>>>>>> +    if (!prm->fb_ep_enabled) {
>>>>
>>>> prm->fb_ep_enabled is not protected. Potential race problem here?
>>>
>>> Given how the variable is used, I don't think so.
>>> Could you please detail ?
>>>
>>
>> I'm thinking of this scenario:
>>
>> Since free_ep_fback() is called in a separate thread,
>> u_audio_iso_fback_complete() can come in the middle after
>> prm->fb_ep_enabled is cleared but before the usb_ep_dequeue(). So the
>> request may be freed before being accessed again in usb_ep_dequeue().
>>
> 
> You are right - there is short window of opportunity.
> Looks like a 2nd separate issue affecting all u_audio endpoints. (Would
> be solved by reverting the feedback series)
> 
> Fix seems fairly simple.
> 
>>> (I don't think this is really related to the current problem though)
>>
>> It's related to how we go about to solve the problem right?
>>
>> Can we just check for the req->status for -ECONNRESET and -ESHUTDOWN and
>> free the request? Those 2 statuses should be for when the request is
>> cancelled and when the endpoint is disabled.
>>
> 
> The same problem happened on the data endpoint while feedback was being
> reviewed. This is why data got fixed, and the feedback endpoint did not.
> 
> As explained in the description, the fix is merely aligning what is
> done for the different endpoints here.

IMHO, it'd be better if we can have a proper fix to the newly introduced
feedback endpoint so that doesn't leave problems to be patched up later.
This makes it easier to backport fix patches that don't touch too many
places.

> 
> Some of the people who have worked on this are no USB expert (myself
> included) so I'm sure things can be improved. I'd suggest to take the
> rework u_audio as a 2nd step.

Sure. I just wanted to bring them up as I notice them while looking
through the reported patch.

> 
>>>
>>>>
>>>>>> +        kfree(req->buf);
>>>>>> +        usb_ep_free_request(ep, req);
>>>>>> +        return;
>>>>>> +    }
>>>>>> +
>>>>>> +    if (req->status == -ESHUTDOWN)
>>>>>>           return;
>>>>>>         /*
>>>>>> @@ -421,9 +427,10 @@ static inline void free_ep_fback(struct
>>>>>> uac_rtd_params *prm, struct usb_ep *ep)
>>>>>>       prm->fb_ep_enabled = false;
>>>>>>         if (prm->req_fback) {
>>>>>> -        usb_ep_dequeue(ep, prm->req_fback);
>>>>>> -        kfree(prm->req_fback->buf);
>>>>>> -        usb_ep_free_request(ep, prm->req_fback);
>>>>>> +        if (usb_ep_dequeue(ep, prm->req_fback)) {
>>>>>> +            kfree(prm->req_fback->buf);
>>>>>> +            usb_ep_free_request(ep, prm->req_fback);
>>>>>> +        }
>>>>>>           prm->req_fback = NULL;
>>>>>>       }
>>>>>>   
>>>>
>>>> On a separate note, I notice that f_uac2 only queues a single feedback
>>>> request at a time for isoc endpoint? Even though the interval is 1ms,
>>>> this will easily cause data drop.
>>>>
>>>> Also, you're ignoring other request error status and still processing
>>>> bogus data on request completion? That doesn't seem right.
>>>
>>> Gadget is sendind the feedback data, not processing it. Every data sent
>>
>> Ah ok.. I missed that it's IN request.
>>
>>> is OK.  Yes, packets can be missed with the current implementation,
>>> meaning the feedback value is not reported as often as initially
>>> intended. On slower HW, packets are also missed with 2 requests queued,
>>
>> 2 is still too low. From our HW testing, minimum should be 4 to minimize
>> data loss at 1ms interval.
> 
> Maybe 4 should be the default then ?

Yes. Would it be a problem?

> 
>>
>>> not only on the feedback endpoint but also on the playback
>>> endpoint. Picking the approriate value is not straight forward. For the
>>> feedback endpoint it isn't big deal because, according to the spec, if
>>> the feedback is not sent, the host shall assume the value hasn't
>>> changed. This why the whole thing works as it is.
>>
>> Why do this when we can avoid it.
> 
> Because it was not known at the time
> 
>> We know that there will be data drop
>> with the way it's implemented now. There's no option to adjust the
>> number of feedback requests either.
>>
>>>
>>> I admit things still aren't perfect, but there is progress ...
>>>
>> Thanks for the updates to audio/f_uac2. It's been a long time since
>> anyone working on it.
>>

Thanks,
Thinh

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop
  2021-08-27  0:50         ` Thinh Nguyen
@ 2021-08-27  8:38           ` Jerome Brunet
  2021-08-27 23:30             ` Thinh Nguyen
  0 siblings, 1 reply; 11+ messages in thread
From: Jerome Brunet @ 2021-08-27  8:38 UTC (permalink / raw)
  To: Thinh Nguyen, Ferry Toth, Ferry Toth, Ruslan Bilovol,
	Felipe Balbi, Pavel Hofman
  Cc: Greg Kroah-Hartman, Jack Pham, linux-kernel, linux-usb


On Fri 27 Aug 2021 at 00:50, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:

>>>>> diff --git a/drivers/usb/gadget/function/u_audio.c
>>>>> b/drivers/usb/gadget/function/u_audio.c
>>>>> index 018dd0978995..63d9340f008e 100644
>>>>> --- a/drivers/usb/gadget/function/u_audio.c
>>>>> +++ b/drivers/usb/gadget/function/u_audio.c
>>>>> @@ -230,7 +230,13 @@ static void u_audio_iso_fback_complete(struct
>>>>> usb_ep *ep,
>>>>>       int status = req->status;
>>>>>         /* i/f shutting down */
>>>>> -    if (!prm->fb_ep_enabled || req->status == -ESHUTDOWN)
>>>>> +    if (!prm->fb_ep_enabled) {
>>>
>>> prm->fb_ep_enabled is not protected. Potential race problem here?
>> 
>> Given how the variable is used, I don't think so.
>> Could you please detail ?
>> 
>
> I'm thinking of this scenario:
>
> Since free_ep_fback() is called in a separate thread,
> u_audio_iso_fback_complete() can come in the middle after
> prm->fb_ep_enabled is cleared but before the usb_ep_dequeue(). So the
> request may be freed before being accessed again in usb_ep_dequeue().
>

You are right - there is short window of opportunity.
Looks like a 2nd separate issue affecting all u_audio endpoints. (Would
be solved by reverting the feedback series)

Fix seems fairly simple.

>> (I don't think this is really related to the current problem though)
>
> It's related to how we go about to solve the problem right?
>
> Can we just check for the req->status for -ECONNRESET and -ESHUTDOWN and
> free the request? Those 2 statuses should be for when the request is
> cancelled and when the endpoint is disabled.
>

The same problem happened on the data endpoint while feedback was being
reviewed. This is why data got fixed, and the feedback endpoint did not.

As explained in the description, the fix is merely aligning what is
done for the different endpoints here.

Some of the people who have worked on this are no USB expert (myself
included) so I'm sure things can be improved. I'd suggest to take the
rework u_audio as a 2nd step.

>> 
>>>
>>>>> +        kfree(req->buf);
>>>>> +        usb_ep_free_request(ep, req);
>>>>> +        return;
>>>>> +    }
>>>>> +
>>>>> +    if (req->status == -ESHUTDOWN)
>>>>>           return;
>>>>>         /*
>>>>> @@ -421,9 +427,10 @@ static inline void free_ep_fback(struct
>>>>> uac_rtd_params *prm, struct usb_ep *ep)
>>>>>       prm->fb_ep_enabled = false;
>>>>>         if (prm->req_fback) {
>>>>> -        usb_ep_dequeue(ep, prm->req_fback);
>>>>> -        kfree(prm->req_fback->buf);
>>>>> -        usb_ep_free_request(ep, prm->req_fback);
>>>>> +        if (usb_ep_dequeue(ep, prm->req_fback)) {
>>>>> +            kfree(prm->req_fback->buf);
>>>>> +            usb_ep_free_request(ep, prm->req_fback);
>>>>> +        }
>>>>>           prm->req_fback = NULL;
>>>>>       }
>>>>>   
>>>
>>> On a separate note, I notice that f_uac2 only queues a single feedback
>>> request at a time for isoc endpoint? Even though the interval is 1ms,
>>> this will easily cause data drop.
>>>
>>> Also, you're ignoring other request error status and still processing
>>> bogus data on request completion? That doesn't seem right.
>> 
>> Gadget is sendind the feedback data, not processing it. Every data sent
>
> Ah ok.. I missed that it's IN request.
>
>> is OK.  Yes, packets can be missed with the current implementation,
>> meaning the feedback value is not reported as often as initially
>> intended. On slower HW, packets are also missed with 2 requests queued,
>
> 2 is still too low. From our HW testing, minimum should be 4 to minimize
> data loss at 1ms interval.

Maybe 4 should be the default then ?

>
>> not only on the feedback endpoint but also on the playback
>> endpoint. Picking the approriate value is not straight forward. For the
>> feedback endpoint it isn't big deal because, according to the spec, if
>> the feedback is not sent, the host shall assume the value hasn't
>> changed. This why the whole thing works as it is.
>
> Why do this when we can avoid it.

Because it was not known at the time

> We know that there will be data drop
> with the way it's implemented now. There's no option to adjust the
> number of feedback requests either.
>
>> 
>> I admit things still aren't perfect, but there is progress ...
>> 
> Thanks for the updates to audio/f_uac2. It's been a long time since
> anyone working on it.
>
> BR,
> Thinh


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop
  2021-08-26  7:25       ` Jerome Brunet
@ 2021-08-27  0:50         ` Thinh Nguyen
  2021-08-27  8:38           ` Jerome Brunet
  0 siblings, 1 reply; 11+ messages in thread
From: Thinh Nguyen @ 2021-08-27  0:50 UTC (permalink / raw)
  To: Jerome Brunet, Thinh Nguyen, Ferry Toth, Ferry Toth,
	Ruslan Bilovol, Felipe Balbi, Pavel Hofman
  Cc: Greg Kroah-Hartman, Jack Pham, linux-kernel, linux-usb

Jerome Brunet wrote:
> 
> On Wed 25 Aug 2021 at 21:42, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
> 
>> Ferry Toth wrote:
>>> Hi,
>>>
>>> Op 25-08-2021 om 11:21 schreef Jerome Brunet:
>>>> When the uac2 function is stopped, there seems to be an issue with some
>>>> platforms (Intel Merrifield at least)
>>>>
>>
>> The issue isn't hardware specific.
> 
> While the actual bug isnt, the report was (given the issue did not show
> up during initial testing but did show on Ferry's HW)
> Merely citing the bug report from Ferry here

Ok

> 
>>
>>>> BUG: kernel NULL pointer dereference, address: 0000000000000008
>>>> ...
>>>> RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
>>>> ...
>>>> Call Trace:
>>>>   dwc3_remove_requests.constprop.0+0x12f/0x170
>>>>   __dwc3_gadget_ep_disable+0x7a/0x160
>>>>   dwc3_gadget_ep_disable+0x3d/0xd0
>>>>   usb_ep_disable+0x1c/0x70
>>>>   u_audio_stop_capture+0x79/0x120 [u_audio]
>>>>   afunc_set_alt+0x73/0x80 [usb_f_uac2]
>>>>   composite_setup+0x224/0x1b90 [libcomposite]
>>>>
>>>> The issue happens only when the gadget is using the sync type "async",
>>>> not
>>>> "adaptive". This indicates that problem is likely coming from the
>>>> feedback
>>>> endpoint, which is only used with async synchronization mode.
>>
>> This does not describe the actual problem. The problem is that the
>> usb_ep_dequeue() can be an asynchronous call, and we can't free the
>> request until its completion (from cancellation).
> 
> Indeed. I was not sure at the time.
> 
>>
>>>>
>>>> Update the feedback endpoint free function to release the endpoint the
>>>> same
>>>> way it is done for the data endpoint.
>>>>
>>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>>> ---
>>>>
>>>>   Hi Ferry,
>>>>
>>>>   Would you mind trying this before reverting the whole thing ?
>>>>   The HW I have did not show the issue so far so I can't really check
>>>>   if it helps. Hopefully, it does ...
>>>
>>> Tested this evening and confirming that this resolves my issue. I can't
>>> say much about the code itself, maybe Thinh?
>>
>> Sure. I can take a look.
>>
>>>
>>> Would be great if we could get this in instead of reverting the series.
>>>
>>> Tested-by:  Ferry Toth <ftoth@exalondelft.nl> (dwc3 / Intel Merrifield)
>>>
>>>>   drivers/usb/gadget/function/u_audio.c | 15 +++++++++++----
>>>>   1 file changed, 11 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/gadget/function/u_audio.c
>>>> b/drivers/usb/gadget/function/u_audio.c
>>>> index 018dd0978995..63d9340f008e 100644
>>>> --- a/drivers/usb/gadget/function/u_audio.c
>>>> +++ b/drivers/usb/gadget/function/u_audio.c
>>>> @@ -230,7 +230,13 @@ static void u_audio_iso_fback_complete(struct
>>>> usb_ep *ep,
>>>>       int status = req->status;
>>>>         /* i/f shutting down */
>>>> -    if (!prm->fb_ep_enabled || req->status == -ESHUTDOWN)
>>>> +    if (!prm->fb_ep_enabled) {
>>
>> prm->fb_ep_enabled is not protected. Potential race problem here?
> 
> Given how the variable is used, I don't think so.
> Could you please detail ?
> 

I'm thinking of this scenario:

Since free_ep_fback() is called in a separate thread,
u_audio_iso_fback_complete() can come in the middle after
prm->fb_ep_enabled is cleared but before the usb_ep_dequeue(). So the
request may be freed before being accessed again in usb_ep_dequeue().

> (I don't think this is really related to the current problem though)

It's related to how we go about to solve the problem right?

Can we just check for the req->status for -ECONNRESET and -ESHUTDOWN and
free the request? Those 2 statuses should be for when the request is
cancelled and when the endpoint is disabled.

> 
>>
>>>> +        kfree(req->buf);
>>>> +        usb_ep_free_request(ep, req);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    if (req->status == -ESHUTDOWN)
>>>>           return;
>>>>         /*
>>>> @@ -421,9 +427,10 @@ static inline void free_ep_fback(struct
>>>> uac_rtd_params *prm, struct usb_ep *ep)
>>>>       prm->fb_ep_enabled = false;
>>>>         if (prm->req_fback) {
>>>> -        usb_ep_dequeue(ep, prm->req_fback);
>>>> -        kfree(prm->req_fback->buf);
>>>> -        usb_ep_free_request(ep, prm->req_fback);
>>>> +        if (usb_ep_dequeue(ep, prm->req_fback)) {
>>>> +            kfree(prm->req_fback->buf);
>>>> +            usb_ep_free_request(ep, prm->req_fback);
>>>> +        }
>>>>           prm->req_fback = NULL;
>>>>       }
>>>>   
>>
>> On a separate note, I notice that f_uac2 only queues a single feedback
>> request at a time for isoc endpoint? Even though the interval is 1ms,
>> this will easily cause data drop.
>>
>> Also, you're ignoring other request error status and still processing
>> bogus data on request completion? That doesn't seem right.
> 
> Gadget is sendind the feedback data, not processing it. Every data sent

Ah ok.. I missed that it's IN request.

> is OK.  Yes, packets can be missed with the current implementation,
> meaning the feedback value is not reported as often as initially
> intended. On slower HW, packets are also missed with 2 requests queued,

2 is still too low. From our HW testing, minimum should be 4 to minimize
data loss at 1ms interval.

> not only on the feedback endpoint but also on the playback
> endpoint. Picking the approriate value is not straight forward. For the
> feedback endpoint it isn't big deal because, according to the spec, if
> the feedback is not sent, the host shall assume the value hasn't
> changed. This why the whole thing works as it is.

Why do this when we can avoid it. We know that there will be data drop
with the way it's implemented now. There's no option to adjust the
number of feedback requests either.

> 
> I admit things still aren't perfect, but there is progress ...
> 
Thanks for the updates to audio/f_uac2. It's been a long time since
anyone working on it.

BR,
Thinh

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop
  2021-08-25 21:42     ` Thinh Nguyen
@ 2021-08-26  7:25       ` Jerome Brunet
  2021-08-27  0:50         ` Thinh Nguyen
  0 siblings, 1 reply; 11+ messages in thread
From: Jerome Brunet @ 2021-08-26  7:25 UTC (permalink / raw)
  To: Thinh Nguyen, Ferry Toth, Ferry Toth, Ruslan Bilovol,
	Felipe Balbi, Pavel Hofman
  Cc: Greg Kroah-Hartman, Jack Pham, linux-kernel, linux-usb


On Wed 25 Aug 2021 at 21:42, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:

> Ferry Toth wrote:
>> Hi,
>> 
>> Op 25-08-2021 om 11:21 schreef Jerome Brunet:
>>> When the uac2 function is stopped, there seems to be an issue with some
>>> platforms (Intel Merrifield at least)
>>>
>
> The issue isn't hardware specific.

While the actual bug isnt, the report was (given the issue did not show
up during initial testing but did show on Ferry's HW)
Merely citing the bug report from Ferry here

>
>>> BUG: kernel NULL pointer dereference, address: 0000000000000008
>>> ...
>>> RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
>>> ...
>>> Call Trace:
>>>   dwc3_remove_requests.constprop.0+0x12f/0x170
>>>   __dwc3_gadget_ep_disable+0x7a/0x160
>>>   dwc3_gadget_ep_disable+0x3d/0xd0
>>>   usb_ep_disable+0x1c/0x70
>>>   u_audio_stop_capture+0x79/0x120 [u_audio]
>>>   afunc_set_alt+0x73/0x80 [usb_f_uac2]
>>>   composite_setup+0x224/0x1b90 [libcomposite]
>>>
>>> The issue happens only when the gadget is using the sync type "async",
>>> not
>>> "adaptive". This indicates that problem is likely coming from the
>>> feedback
>>> endpoint, which is only used with async synchronization mode.
>
> This does not describe the actual problem. The problem is that the
> usb_ep_dequeue() can be an asynchronous call, and we can't free the
> request until its completion (from cancellation).

Indeed. I was not sure at the time.

>
>>>
>>> Update the feedback endpoint free function to release the endpoint the
>>> same
>>> way it is done for the data endpoint.
>>>
>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>> ---
>>>
>>>   Hi Ferry,
>>>
>>>   Would you mind trying this before reverting the whole thing ?
>>>   The HW I have did not show the issue so far so I can't really check
>>>   if it helps. Hopefully, it does ...
>> 
>> Tested this evening and confirming that this resolves my issue. I can't
>> say much about the code itself, maybe Thinh?
>
> Sure. I can take a look.
>
>> 
>> Would be great if we could get this in instead of reverting the series.
>> 
>> Tested-by:  Ferry Toth <ftoth@exalondelft.nl> (dwc3 / Intel Merrifield)
>> 
>>>   drivers/usb/gadget/function/u_audio.c | 15 +++++++++++----
>>>   1 file changed, 11 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/usb/gadget/function/u_audio.c
>>> b/drivers/usb/gadget/function/u_audio.c
>>> index 018dd0978995..63d9340f008e 100644
>>> --- a/drivers/usb/gadget/function/u_audio.c
>>> +++ b/drivers/usb/gadget/function/u_audio.c
>>> @@ -230,7 +230,13 @@ static void u_audio_iso_fback_complete(struct
>>> usb_ep *ep,
>>>       int status = req->status;
>>>         /* i/f shutting down */
>>> -    if (!prm->fb_ep_enabled || req->status == -ESHUTDOWN)
>>> +    if (!prm->fb_ep_enabled) {
>
> prm->fb_ep_enabled is not protected. Potential race problem here?

Given how the variable is used, I don't think so.
Could you please detail ?

(I don't think this is really related to the current problem though)

>
>>> +        kfree(req->buf);
>>> +        usb_ep_free_request(ep, req);
>>> +        return;
>>> +    }
>>> +
>>> +    if (req->status == -ESHUTDOWN)
>>>           return;
>>>         /*
>>> @@ -421,9 +427,10 @@ static inline void free_ep_fback(struct
>>> uac_rtd_params *prm, struct usb_ep *ep)
>>>       prm->fb_ep_enabled = false;
>>>         if (prm->req_fback) {
>>> -        usb_ep_dequeue(ep, prm->req_fback);
>>> -        kfree(prm->req_fback->buf);
>>> -        usb_ep_free_request(ep, prm->req_fback);
>>> +        if (usb_ep_dequeue(ep, prm->req_fback)) {
>>> +            kfree(prm->req_fback->buf);
>>> +            usb_ep_free_request(ep, prm->req_fback);
>>> +        }
>>>           prm->req_fback = NULL;
>>>       }
>>>   
>
> On a separate note, I notice that f_uac2 only queues a single feedback
> request at a time for isoc endpoint? Even though the interval is 1ms,
> this will easily cause data drop.
>
> Also, you're ignoring other request error status and still processing
> bogus data on request completion? That doesn't seem right.

Gadget is sendind the feedback data, not processing it. Every data sent
is OK.  Yes, packets can be missed with the current implementation,
meaning the feedback value is not reported as often as initially
intended. On slower HW, packets are also missed with 2 requests queued,
not only on the feedback endpoint but also on the playback
endpoint. Picking the approriate value is not straight forward. For the
feedback endpoint it isn't big deal because, according to the spec, if
the feedback is not sent, the host shall assume the value hasn't
changed. This why the whole thing works as it is.

I admit things still aren't perfect, but there is progress ...

>
> BR,
> Thinh


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop
  2021-08-25 20:07   ` Ferry Toth
  2021-08-25 21:42     ` Thinh Nguyen
@ 2021-08-26  7:23     ` Jerome Brunet
  1 sibling, 0 replies; 11+ messages in thread
From: Jerome Brunet @ 2021-08-26  7:23 UTC (permalink / raw)
  To: Ferry Toth, Ferry Toth, Ruslan Bilovol, Felipe Balbi, Pavel Hofman
  Cc: Greg Kroah-Hartman, Jack Pham, linux-kernel, linux-usb, Thinh Nguyen


On Wed 25 Aug 2021 at 22:07, Ferry Toth <fntoth@gmail.com> wrote:

> Hi,
>
> Op 25-08-2021 om 11:21 schreef Jerome Brunet:
>> When the uac2 function is stopped, there seems to be an issue with some
>> platforms (Intel Merrifield at least)
>>
>> BUG: kernel NULL pointer dereference, address: 0000000000000008
>> ...
>> RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
>> ...
>> Call Trace:
>>   dwc3_remove_requests.constprop.0+0x12f/0x170
>>   __dwc3_gadget_ep_disable+0x7a/0x160
>>   dwc3_gadget_ep_disable+0x3d/0xd0
>>   usb_ep_disable+0x1c/0x70
>>   u_audio_stop_capture+0x79/0x120 [u_audio]
>>   afunc_set_alt+0x73/0x80 [usb_f_uac2]
>>   composite_setup+0x224/0x1b90 [libcomposite]
>>
>> The issue happens only when the gadget is using the sync type "async", not
>> "adaptive". This indicates that problem is likely coming from the feedback
>> endpoint, which is only used with async synchronization mode.
>>
>> Update the feedback endpoint free function to release the endpoint the same
>> way it is done for the data endpoint.
>>
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>>
>>   Hi Ferry,
>>
>>   Would you mind trying this before reverting the whole thing ?
>>   The HW I have did not show the issue so far so I can't really check
>>   if it helps. Hopefully, it does ...
>
> Tested this evening and confirming that this resolves my issue. I can't say
> much about the code itself, maybe Thinh?
>
> Would be great if we could get this in instead of reverting the series.
>
> Tested-by:  Ferry Toth <ftoth@exalondelft.nl> (dwc3 / Intel Merrifield)
>

Thanks for reporting and testing this.
I guess we should also add

Reported-by: Ferry Toth <ftoth@exalondelft.nl>


>>   drivers/usb/gadget/function/u_audio.c | 15 +++++++++++----
>>   1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
>> index 018dd0978995..63d9340f008e 100644
>> --- a/drivers/usb/gadget/function/u_audio.c
>> +++ b/drivers/usb/gadget/function/u_audio.c
>> @@ -230,7 +230,13 @@ static void u_audio_iso_fback_complete(struct usb_ep *ep,
>>   	int status = req->status;
>>     	/* i/f shutting down */
>> -	if (!prm->fb_ep_enabled || req->status == -ESHUTDOWN)
>> +	if (!prm->fb_ep_enabled) {
>> +		kfree(req->buf);
>> +		usb_ep_free_request(ep, req);
>> +		return;
>> +	}
>> +
>> +	if (req->status == -ESHUTDOWN)
>>   		return;
>>     	/*
>> @@ -421,9 +427,10 @@ static inline void free_ep_fback(struct uac_rtd_params *prm, struct usb_ep *ep)
>>   	prm->fb_ep_enabled = false;
>>     	if (prm->req_fback) {
>> -		usb_ep_dequeue(ep, prm->req_fback);
>> -		kfree(prm->req_fback->buf);
>> -		usb_ep_free_request(ep, prm->req_fback);
>> +		if (usb_ep_dequeue(ep, prm->req_fback)) {
>> +			kfree(prm->req_fback->buf);
>> +			usb_ep_free_request(ep, prm->req_fback);
>> +		}
>>   		prm->req_fback = NULL;
>>   	}
>>   


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop
  2021-08-25 20:07   ` Ferry Toth
@ 2021-08-25 21:42     ` Thinh Nguyen
  2021-08-26  7:25       ` Jerome Brunet
  2021-08-26  7:23     ` Jerome Brunet
  1 sibling, 1 reply; 11+ messages in thread
From: Thinh Nguyen @ 2021-08-25 21:42 UTC (permalink / raw)
  To: Ferry Toth, Jerome Brunet, Ferry Toth, Ruslan Bilovol,
	Felipe Balbi, Pavel Hofman
  Cc: Greg Kroah-Hartman, Jack Pham, linux-kernel, linux-usb, Thinh Nguyen

Ferry Toth wrote:
> Hi,
> 
> Op 25-08-2021 om 11:21 schreef Jerome Brunet:
>> When the uac2 function is stopped, there seems to be an issue with some
>> platforms (Intel Merrifield at least)
>>

The issue isn't hardware specific.

>> BUG: kernel NULL pointer dereference, address: 0000000000000008
>> ...
>> RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
>> ...
>> Call Trace:
>>   dwc3_remove_requests.constprop.0+0x12f/0x170
>>   __dwc3_gadget_ep_disable+0x7a/0x160
>>   dwc3_gadget_ep_disable+0x3d/0xd0
>>   usb_ep_disable+0x1c/0x70
>>   u_audio_stop_capture+0x79/0x120 [u_audio]
>>   afunc_set_alt+0x73/0x80 [usb_f_uac2]
>>   composite_setup+0x224/0x1b90 [libcomposite]
>>
>> The issue happens only when the gadget is using the sync type "async",
>> not
>> "adaptive". This indicates that problem is likely coming from the
>> feedback
>> endpoint, which is only used with async synchronization mode.

This does not describe the actual problem. The problem is that the
usb_ep_dequeue() can be an asynchronous call, and we can't free the
request until its completion (from cancellation).

>>
>> Update the feedback endpoint free function to release the endpoint the
>> same
>> way it is done for the data endpoint.
>>
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>>
>>   Hi Ferry,
>>
>>   Would you mind trying this before reverting the whole thing ?
>>   The HW I have did not show the issue so far so I can't really check
>>   if it helps. Hopefully, it does ...
> 
> Tested this evening and confirming that this resolves my issue. I can't
> say much about the code itself, maybe Thinh?

Sure. I can take a look.

> 
> Would be great if we could get this in instead of reverting the series.
> 
> Tested-by:  Ferry Toth <ftoth@exalondelft.nl> (dwc3 / Intel Merrifield)
> 
>>   drivers/usb/gadget/function/u_audio.c | 15 +++++++++++----
>>   1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/u_audio.c
>> b/drivers/usb/gadget/function/u_audio.c
>> index 018dd0978995..63d9340f008e 100644
>> --- a/drivers/usb/gadget/function/u_audio.c
>> +++ b/drivers/usb/gadget/function/u_audio.c
>> @@ -230,7 +230,13 @@ static void u_audio_iso_fback_complete(struct
>> usb_ep *ep,
>>       int status = req->status;
>>         /* i/f shutting down */
>> -    if (!prm->fb_ep_enabled || req->status == -ESHUTDOWN)
>> +    if (!prm->fb_ep_enabled) {

prm->fb_ep_enabled is not protected. Potential race problem here?

>> +        kfree(req->buf);
>> +        usb_ep_free_request(ep, req);
>> +        return;
>> +    }
>> +
>> +    if (req->status == -ESHUTDOWN)
>>           return;
>>         /*
>> @@ -421,9 +427,10 @@ static inline void free_ep_fback(struct
>> uac_rtd_params *prm, struct usb_ep *ep)
>>       prm->fb_ep_enabled = false;
>>         if (prm->req_fback) {
>> -        usb_ep_dequeue(ep, prm->req_fback);
>> -        kfree(prm->req_fback->buf);
>> -        usb_ep_free_request(ep, prm->req_fback);
>> +        if (usb_ep_dequeue(ep, prm->req_fback)) {
>> +            kfree(prm->req_fback->buf);
>> +            usb_ep_free_request(ep, prm->req_fback);
>> +        }
>>           prm->req_fback = NULL;
>>       }
>>   

On a separate note, I notice that f_uac2 only queues a single feedback
request at a time for isoc endpoint? Even though the interval is 1ms,
this will easily cause data drop.

Also, you're ignoring other request error status and still processing
bogus data on request completion? That doesn't seem right.

BR,
Thinh

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop
  2021-08-25  9:21 ` [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop Jerome Brunet
@ 2021-08-25 20:07   ` Ferry Toth
  2021-08-25 21:42     ` Thinh Nguyen
  2021-08-26  7:23     ` Jerome Brunet
  0 siblings, 2 replies; 11+ messages in thread
From: Ferry Toth @ 2021-08-25 20:07 UTC (permalink / raw)
  To: Jerome Brunet, Ferry Toth, Ruslan Bilovol, Felipe Balbi, Pavel Hofman
  Cc: Greg Kroah-Hartman, Jack Pham, linux-kernel, linux-usb, Thinh Nguyen

Hi,

Op 25-08-2021 om 11:21 schreef Jerome Brunet:
> When the uac2 function is stopped, there seems to be an issue with some
> platforms (Intel Merrifield at least)
>
> BUG: kernel NULL pointer dereference, address: 0000000000000008
> ...
> RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
> ...
> Call Trace:
>   dwc3_remove_requests.constprop.0+0x12f/0x170
>   __dwc3_gadget_ep_disable+0x7a/0x160
>   dwc3_gadget_ep_disable+0x3d/0xd0
>   usb_ep_disable+0x1c/0x70
>   u_audio_stop_capture+0x79/0x120 [u_audio]
>   afunc_set_alt+0x73/0x80 [usb_f_uac2]
>   composite_setup+0x224/0x1b90 [libcomposite]
>
> The issue happens only when the gadget is using the sync type "async", not
> "adaptive". This indicates that problem is likely coming from the feedback
> endpoint, which is only used with async synchronization mode.
>
> Update the feedback endpoint free function to release the endpoint the same
> way it is done for the data endpoint.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>
>   Hi Ferry,
>
>   Would you mind trying this before reverting the whole thing ?
>   The HW I have did not show the issue so far so I can't really check
>   if it helps. Hopefully, it does ...

Tested this evening and confirming that this resolves my issue. I can't 
say much about the code itself, maybe Thinh?

Would be great if we could get this in instead of reverting the series.

Tested-by:  Ferry Toth <ftoth@exalondelft.nl> (dwc3 / Intel Merrifield)

>   drivers/usb/gadget/function/u_audio.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
> index 018dd0978995..63d9340f008e 100644
> --- a/drivers/usb/gadget/function/u_audio.c
> +++ b/drivers/usb/gadget/function/u_audio.c
> @@ -230,7 +230,13 @@ static void u_audio_iso_fback_complete(struct usb_ep *ep,
>   	int status = req->status;
>   
>   	/* i/f shutting down */
> -	if (!prm->fb_ep_enabled || req->status == -ESHUTDOWN)
> +	if (!prm->fb_ep_enabled) {
> +		kfree(req->buf);
> +		usb_ep_free_request(ep, req);
> +		return;
> +	}
> +
> +	if (req->status == -ESHUTDOWN)
>   		return;
>   
>   	/*
> @@ -421,9 +427,10 @@ static inline void free_ep_fback(struct uac_rtd_params *prm, struct usb_ep *ep)
>   	prm->fb_ep_enabled = false;
>   
>   	if (prm->req_fback) {
> -		usb_ep_dequeue(ep, prm->req_fback);
> -		kfree(prm->req_fback->buf);
> -		usb_ep_free_request(ep, prm->req_fback);
> +		if (usb_ep_dequeue(ep, prm->req_fback)) {
> +			kfree(prm->req_fback->buf);
> +			usb_ep_free_request(ep, prm->req_fback);
> +		}
>   		prm->req_fback = NULL;
>   	}
>   

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop
  2021-08-24 20:14 [PATCH v1 1/3] Revert "usb: gadget: u_audio: add real feedback implementation" Ferry Toth
@ 2021-08-25  9:21 ` Jerome Brunet
  2021-08-25 20:07   ` Ferry Toth
  0 siblings, 1 reply; 11+ messages in thread
From: Jerome Brunet @ 2021-08-25  9:21 UTC (permalink / raw)
  To: Ferry Toth, Ferry Toth, Ruslan Bilovol, Felipe Balbi, Pavel Hofman
  Cc: Jerome Brunet, Greg Kroah-Hartman, Jack Pham, linux-kernel, linux-usb

When the uac2 function is stopped, there seems to be an issue with some
platforms (Intel Merrifield at least)

BUG: kernel NULL pointer dereference, address: 0000000000000008
...
RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
...
Call Trace:
 dwc3_remove_requests.constprop.0+0x12f/0x170
 __dwc3_gadget_ep_disable+0x7a/0x160
 dwc3_gadget_ep_disable+0x3d/0xd0
 usb_ep_disable+0x1c/0x70
 u_audio_stop_capture+0x79/0x120 [u_audio]
 afunc_set_alt+0x73/0x80 [usb_f_uac2]
 composite_setup+0x224/0x1b90 [libcomposite]

The issue happens only when the gadget is using the sync type "async", not
"adaptive". This indicates that problem is likely coming from the feedback
endpoint, which is only used with async synchronization mode.

Update the feedback endpoint free function to release the endpoint the same
way it is done for the data endpoint.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---

 Hi Ferry,

 Would you mind trying this before reverting the whole thing ?
 The HW I have did not show the issue so far so I can't really check
 if it helps. Hopefully, it does ...

 drivers/usb/gadget/function/u_audio.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index 018dd0978995..63d9340f008e 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -230,7 +230,13 @@ static void u_audio_iso_fback_complete(struct usb_ep *ep,
 	int status = req->status;
 
 	/* i/f shutting down */
-	if (!prm->fb_ep_enabled || req->status == -ESHUTDOWN)
+	if (!prm->fb_ep_enabled) {
+		kfree(req->buf);
+		usb_ep_free_request(ep, req);
+		return;
+	}
+
+	if (req->status == -ESHUTDOWN)
 		return;
 
 	/*
@@ -421,9 +427,10 @@ static inline void free_ep_fback(struct uac_rtd_params *prm, struct usb_ep *ep)
 	prm->fb_ep_enabled = false;
 
 	if (prm->req_fback) {
-		usb_ep_dequeue(ep, prm->req_fback);
-		kfree(prm->req_fback->buf);
-		usb_ep_free_request(ep, prm->req_fback);
+		if (usb_ep_dequeue(ep, prm->req_fback)) {
+			kfree(prm->req_fback->buf);
+			usb_ep_free_request(ep, prm->req_fback);
+		}
 		prm->req_fback = NULL;
 	}
 
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-08-27 23:30 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-27  7:58 [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop Jerome Brunet
2021-08-27  8:05 ` Felipe Balbi
2021-08-27  9:26   ` Jerome Brunet
  -- strict thread matches above, loose matches on Subject: below --
2021-08-24 20:14 [PATCH v1 1/3] Revert "usb: gadget: u_audio: add real feedback implementation" Ferry Toth
2021-08-25  9:21 ` [PATCH] usb: gadget: f_uac2: fixup feedback endpoint stop Jerome Brunet
2021-08-25 20:07   ` Ferry Toth
2021-08-25 21:42     ` Thinh Nguyen
2021-08-26  7:25       ` Jerome Brunet
2021-08-27  0:50         ` Thinh Nguyen
2021-08-27  8:38           ` Jerome Brunet
2021-08-27 23:30             ` Thinh Nguyen
2021-08-26  7:23     ` Jerome Brunet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).