* [PATCH 1/2] usb: gadget: f_uac2: Stop endpoint before enabling it.
@ 2021-04-19 7:50 Pawel Laszczak
2021-04-20 1:08 ` Peter Chen
0 siblings, 1 reply; 5+ messages in thread
From: Pawel Laszczak @ 2021-04-19 7:50 UTC (permalink / raw)
To: balbi
Cc: gregkh, ruslan.bilovol, jbrunet, linux-usb, linux-kernel, kurahul,
Pawel Laszczak
From: Pawel Laszczak <pawell@cadence.com>
Patch adds disabling endpoint before enabling it during changing
alternate setting. Lack of this functionality causes that in some
cases uac2 queue the same request multiple time.
Such situation can occur when host send set interface with
alternate setting 1 twice.
Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
drivers/usb/gadget/function/f_uac2.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index 9cc5c512a5cd..7d20a9d8a1b4 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -890,17 +890,17 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
if (intf == uac2->as_out_intf) {
uac2->as_out_alt = alt;
+ u_audio_stop_capture(&uac2->g_audio);
+
if (alt)
ret = u_audio_start_capture(&uac2->g_audio);
- else
- u_audio_stop_capture(&uac2->g_audio);
} else if (intf == uac2->as_in_intf) {
uac2->as_in_alt = alt;
+ u_audio_stop_playback(&uac2->g_audio);
+
if (alt)
ret = u_audio_start_playback(&uac2->g_audio);
- else
- u_audio_stop_playback(&uac2->g_audio);
} else {
dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return -EINVAL;
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] usb: gadget: f_uac2: Stop endpoint before enabling it.
2021-04-19 7:50 [PATCH 1/2] usb: gadget: f_uac2: Stop endpoint before enabling it Pawel Laszczak
@ 2021-04-20 1:08 ` Peter Chen
2021-04-20 3:56 ` Pawel Laszczak
0 siblings, 1 reply; 5+ messages in thread
From: Peter Chen @ 2021-04-20 1:08 UTC (permalink / raw)
To: Pawel Laszczak
Cc: balbi, gregkh, ruslan.bilovol, jbrunet, linux-usb, linux-kernel,
kurahul
On 21-04-19 09:50:53, Pawel Laszczak wrote:
> From: Pawel Laszczak <pawell@cadence.com>
>
> Patch adds disabling endpoint before enabling it during changing
> alternate setting. Lack of this functionality causes that in some
> cases uac2 queue the same request multiple time.
> Such situation can occur when host send set interface with
> alternate setting 1 twice.
>
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
> drivers/usb/gadget/function/f_uac2.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
> index 9cc5c512a5cd..7d20a9d8a1b4 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -890,17 +890,17 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
> if (intf == uac2->as_out_intf) {
> uac2->as_out_alt = alt;
>
> + u_audio_stop_capture(&uac2->g_audio);
> +
> if (alt)
> ret = u_audio_start_capture(&uac2->g_audio);
> - else
> - u_audio_stop_capture(&uac2->g_audio);
> } else if (intf == uac2->as_in_intf) {
> uac2->as_in_alt = alt;
>
> + u_audio_stop_playback(&uac2->g_audio);
> +
> if (alt)
> ret = u_audio_start_playback(&uac2->g_audio);
> - else
> - u_audio_stop_playback(&uac2->g_audio);
> } else {
> dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> return -EINVAL;
To avoid this, you may use prm->ep_enabled to judge if the endpoint has
already enabled.
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH 1/2] usb: gadget: f_uac2: Stop endpoint before enabling it.
2021-04-20 1:08 ` Peter Chen
@ 2021-04-20 3:56 ` Pawel Laszczak
2021-04-23 10:49 ` Peter Chen
0 siblings, 1 reply; 5+ messages in thread
From: Pawel Laszczak @ 2021-04-20 3:56 UTC (permalink / raw)
To: Peter Chen
Cc: balbi@kernel.org, gregkh@linuxfoundation.org,
ruslan.bilovol@gmail.com, jbrunet@baylibre.com,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
Rahul Kumar
>On 21-04-19 09:50:53, Pawel Laszczak wrote:
>> From: Pawel Laszczak <pawell@cadence.com>
>>
>> Patch adds disabling endpoint before enabling it during changing
>> alternate setting. Lack of this functionality causes that in some
>> cases uac2 queue the same request multiple time.
>> Such situation can occur when host send set interface with
>> alternate setting 1 twice.
>>
>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>> ---
>> drivers/usb/gadget/function/f_uac2.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
>> index 9cc5c512a5cd..7d20a9d8a1b4 100644
>> --- a/drivers/usb/gadget/function/f_uac2.c
>> +++ b/drivers/usb/gadget/function/f_uac2.c
>> @@ -890,17 +890,17 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
>> if (intf == uac2->as_out_intf) {
>> uac2->as_out_alt = alt;
>>
>> + u_audio_stop_capture(&uac2->g_audio);
>> +
>> if (alt)
>> ret = u_audio_start_capture(&uac2->g_audio);
>> - else
>> - u_audio_stop_capture(&uac2->g_audio);
>> } else if (intf == uac2->as_in_intf) {
>> uac2->as_in_alt = alt;
>>
>> + u_audio_stop_playback(&uac2->g_audio);
>> +
>> if (alt)
>> ret = u_audio_start_playback(&uac2->g_audio);
>> - else
>> - u_audio_stop_playback(&uac2->g_audio);
>> } else {
>> dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
>> return -EINVAL;
>
>To avoid this, you may use prm->ep_enabled to judge if the endpoint has
>already enabled.
Such condition is as first instruction inside u_audio_stop_playback->free_ep function,
so we don't need duplicate it here.
>
>--
>
>Thanks,
>Peter Chen
--
Regards,
Pawe Laszczak
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] usb: gadget: f_uac2: Stop endpoint before enabling it.
2021-04-20 3:56 ` Pawel Laszczak
@ 2021-04-23 10:49 ` Peter Chen
2021-04-25 3:28 ` Peter Chen
0 siblings, 1 reply; 5+ messages in thread
From: Peter Chen @ 2021-04-23 10:49 UTC (permalink / raw)
To: Pawel Laszczak
Cc: balbi@kernel.org, gregkh@linuxfoundation.org,
ruslan.bilovol@gmail.com, jbrunet@baylibre.com,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
Rahul Kumar
On 21-04-20 03:56:25, Pawel Laszczak wrote:
> >On 21-04-19 09:50:53, Pawel Laszczak wrote:
> >> From: Pawel Laszczak <pawell@cadence.com>
> >>
> >> Patch adds disabling endpoint before enabling it during changing
> >> alternate setting. Lack of this functionality causes that in some
> >> cases uac2 queue the same request multiple time.
> >> Such situation can occur when host send set interface with
> >> alternate setting 1 twice.
> >>
> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> >> ---
> >> drivers/usb/gadget/function/f_uac2.c | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
> >> index 9cc5c512a5cd..7d20a9d8a1b4 100644
> >> --- a/drivers/usb/gadget/function/f_uac2.c
> >> +++ b/drivers/usb/gadget/function/f_uac2.c
> >> @@ -890,17 +890,17 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
> >> if (intf == uac2->as_out_intf) {
> >> uac2->as_out_alt = alt;
> >>
> >> + u_audio_stop_capture(&uac2->g_audio);
> >> +
> >> if (alt)
> >> ret = u_audio_start_capture(&uac2->g_audio);
> >> - else
> >> - u_audio_stop_capture(&uac2->g_audio);
> >> } else if (intf == uac2->as_in_intf) {
> >> uac2->as_in_alt = alt;
> >>
> >> + u_audio_stop_playback(&uac2->g_audio);
> >> +
> >> if (alt)
> >> ret = u_audio_start_playback(&uac2->g_audio);
> >> - else
> >> - u_audio_stop_playback(&uac2->g_audio);
> >> } else {
> >> dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> >> return -EINVAL;
> >
> >To avoid this, you may use prm->ep_enabled to judge if the endpoint has
> >already enabled.
>
> Such condition is as first instruction inside u_audio_stop_playback->free_ep function,
> so we don't need duplicate it here.
>
Get your points, you may add more explanations both the code your change
and the commit log.
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] usb: gadget: f_uac2: Stop endpoint before enabling it.
2021-04-23 10:49 ` Peter Chen
@ 2021-04-25 3:28 ` Peter Chen
0 siblings, 0 replies; 5+ messages in thread
From: Peter Chen @ 2021-04-25 3:28 UTC (permalink / raw)
To: Pawel Laszczak
Cc: balbi@kernel.org, gregkh@linuxfoundation.org,
ruslan.bilovol@gmail.com, jbrunet@baylibre.com,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
Rahul Kumar
On 21-04-23 18:49:14, Peter Chen wrote:
> On 21-04-20 03:56:25, Pawel Laszczak wrote:
> > >On 21-04-19 09:50:53, Pawel Laszczak wrote:
> > >> From: Pawel Laszczak <pawell@cadence.com>
> > >>
> > >> Patch adds disabling endpoint before enabling it during changing
> > >> alternate setting. Lack of this functionality causes that in some
> > >> cases uac2 queue the same request multiple time.
> > >> Such situation can occur when host send set interface with
> > >> alternate setting 1 twice.
> > >>
> > >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> > >> ---
> > >> drivers/usb/gadget/function/f_uac2.c | 8 ++++----
> > >> 1 file changed, 4 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
> > >> index 9cc5c512a5cd..7d20a9d8a1b4 100644
> > >> --- a/drivers/usb/gadget/function/f_uac2.c
> > >> +++ b/drivers/usb/gadget/function/f_uac2.c
> > >> @@ -890,17 +890,17 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
> > >> if (intf == uac2->as_out_intf) {
> > >> uac2->as_out_alt = alt;
> > >>
> > >> + u_audio_stop_capture(&uac2->g_audio);
> > >> +
> > >> if (alt)
> > >> ret = u_audio_start_capture(&uac2->g_audio);
> > >> - else
> > >> - u_audio_stop_capture(&uac2->g_audio);
> > >> } else if (intf == uac2->as_in_intf) {
> > >> uac2->as_in_alt = alt;
> > >>
> > >> + u_audio_stop_playback(&uac2->g_audio);
> > >> +
> > >> if (alt)
> > >> ret = u_audio_start_playback(&uac2->g_audio);
> > >> - else
> > >> - u_audio_stop_playback(&uac2->g_audio);
> > >> } else {
> > >> dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> > >> return -EINVAL;
> > >
> > >To avoid this, you may use prm->ep_enabled to judge if the endpoint has
> > >already enabled.
> >
> > Such condition is as first instruction inside u_audio_stop_playback->free_ep function,
> > so we don't need duplicate it here.
> >
>
> Get your points, you may add more explanations both the code your change
> and the commit log.
>
After thinking more, I think it is better add judge code at
u_audio_start_playback/capture, but not change code here. The reader may
not understand the code your change at the first glance.
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-04-25 3:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-04-19 7:50 [PATCH 1/2] usb: gadget: f_uac2: Stop endpoint before enabling it Pawel Laszczak
2021-04-20 1:08 ` Peter Chen
2021-04-20 3:56 ` Pawel Laszczak
2021-04-23 10:49 ` Peter Chen
2021-04-25 3:28 ` Peter Chen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.