1. Patchew
  2. linux
  3. View series

[PATCH] usb: dwc3: ep0: Don't reset resource alloc flag (including ep0)

Michael Grzeschik posted 1 patch 1 year, 5 months ago
There is a newer version of this series
drivers/usb/dwc3/ep0.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH] usb: dwc3: ep0: Don't reset resource alloc flag (including ep0)
Posted by Michael Grzeschik 1 year, 5 months ago 
The DWC3_EP_RESOURCE_ALLOCATED flag ensures that the resource of an
endpoint is only assigned once. Unless the endpoint is reset, don't
clear this flag. Otherwise we may set endpoint resource again, which
prevents the driver from initiate transfer after handling a STALL or
endpoint halt to the control endpoint.

Commit f2e0eee47038 (usb: dwc3: ep0: Don't reset resource alloc flag)
was fixing the initial issue, but did this only for physical ep1. Since
the function dwc3_ep0_stall_and_restart is resetting the flags for both
physical endpoints, this also has to be done for ep0.

Cc: stable@vger.kernel.org
Fixes: b311048c174d ("usb: dwc3: gadget: Rewrite endpoint allocation flow")
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/usb/dwc3/ep0.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index d96ffbe520397..c9533a99e47c8 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -232,7 +232,8 @@ void dwc3_ep0_stall_and_restart(struct dwc3 *dwc)
 	/* stall is always issued on EP0 */
 	dep = dwc->eps[0];
 	__dwc3_gadget_ep_set_halt(dep, 1, false);
-	dep->flags = DWC3_EP_ENABLED;
+	dep->flags &= DWC3_EP_RESOURCE_ALLOCATED;
+	dep->flags |= DWC3_EP_ENABLED;
 	dwc->delayed_status = false;
 
 	if (!list_empty(&dep->pending_list)) {

---
base-commit: 38343be0bf9a7d7ef0d160da5f2db887a0e29b62
change-id: 20240814-dwc3hwep0reset-b4d371873494

Best regards,
-- 
Michael Grzeschik <m.grzeschik@pengutronix.de>
Re: [PATCH] usb: dwc3: ep0: Don't reset resource alloc flag (including ep0)
Posted by Sergey Shtylyov 1 year, 5 months ago 
On 8/14/24 8:39 PM, Michael Grzeschik wrote:

> The DWC3_EP_RESOURCE_ALLOCATED flag ensures that the resource of an
> endpoint is only assigned once. Unless the endpoint is reset, don't
> clear this flag. Otherwise we may set endpoint resource again, which
> prevents the driver from initiate transfer after handling a STALL or
> endpoint halt to the control endpoint.
> 
> Commit f2e0eee47038 (usb: dwc3: ep0: Don't reset resource alloc flag)

   You forgot the double quotes around the summary, the same as you
do in the Fixes tag.

> was fixing the initial issue, but did this only for physical ep1. Since
> the function dwc3_ep0_stall_and_restart is resetting the flags for both
> physical endpoints, this also has to be done for ep0.
> 
> Cc: stable@vger.kernel.org
> Fixes: b311048c174d ("usb: dwc3: gadget: Rewrite endpoint allocation flow")
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/usb/dwc3/ep0.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index d96ffbe520397..c9533a99e47c8 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
> @@ -232,7 +232,8 @@ void dwc3_ep0_stall_and_restart(struct dwc3 *dwc)
>  	/* stall is always issued on EP0 */
>  	dep = dwc->eps[0];
>  	__dwc3_gadget_ep_set_halt(dep, 1, false);
> -	dep->flags = DWC3_EP_ENABLED;
> +	dep->flags &= DWC3_EP_RESOURCE_ALLOCATED;

   No ~ afer &=?

[...]

MBR, Sergey
Re: [PATCH] usb: dwc3: ep0: Don't reset resource alloc flag (including ep0)
Posted by Sergey Shtylyov 1 year, 5 months ago 
On 8/14/24 11:42 PM, Sergey Shtylyov wrote:
[...]

>> The DWC3_EP_RESOURCE_ALLOCATED flag ensures that the resource of an
>> endpoint is only assigned once. Unless the endpoint is reset, don't
>> clear this flag. Otherwise we may set endpoint resource again, which
>> prevents the driver from initiate transfer after handling a STALL or
>> endpoint halt to the control endpoint.
>>
>> Commit f2e0eee47038 (usb: dwc3: ep0: Don't reset resource alloc flag)
> 
>    You forgot the double quotes around the summary, the same as you
> do in the Fixes tag.
> 
>> was fixing the initial issue, but did this only for physical ep1. Since
>> the function dwc3_ep0_stall_and_restart is resetting the flags for both
>> physical endpoints, this also has to be done for ep0.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: b311048c174d ("usb: dwc3: gadget: Rewrite endpoint allocation flow")
>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> ---
>>  drivers/usb/dwc3/ep0.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
>> index d96ffbe520397..c9533a99e47c8 100644
>> --- a/drivers/usb/dwc3/ep0.c
>> +++ b/drivers/usb/dwc3/ep0.c
>> @@ -232,7 +232,8 @@ void dwc3_ep0_stall_and_restart(struct dwc3 *dwc)
>>  	/* stall is always issued on EP0 */
>>  	dep = dwc->eps[0];
>>  	__dwc3_gadget_ep_set_halt(dep, 1, false);
>> -	dep->flags = DWC3_EP_ENABLED;
>> +	dep->flags &= DWC3_EP_RESOURCE_ALLOCATED;
> 
>    No ~ afer &=?

   Sorry, I wasn't attentive enough while reading the patch description... :-/

[...]

MBR, Sergey
Re: [PATCH] usb: dwc3: ep0: Don't reset resource alloc flag (including ep0)
Posted by Thinh Nguyen 1 year, 5 months ago 
Hi Michael,

On Wed, Aug 14, 2024, Sergey Shtylyov wrote:
> On 8/14/24 11:42 PM, Sergey Shtylyov wrote:
> [...]
> 
> >> The DWC3_EP_RESOURCE_ALLOCATED flag ensures that the resource of an
> >> endpoint is only assigned once. Unless the endpoint is reset, don't
> >> clear this flag. Otherwise we may set endpoint resource again, which
> >> prevents the driver from initiate transfer after handling a STALL or
> >> endpoint halt to the control endpoint.
> >>
> >> Commit f2e0eee47038 (usb: dwc3: ep0: Don't reset resource alloc flag)
> > 
> >    You forgot the double quotes around the summary, the same as you
> > do in the Fixes tag.
> > 
> >> was fixing the initial issue, but did this only for physical ep1. Since
> >> the function dwc3_ep0_stall_and_restart is resetting the flags for both
> >> physical endpoints, this also has to be done for ep0.
> >>
> >> Cc: stable@vger.kernel.org
> >> Fixes: b311048c174d ("usb: dwc3: gadget: Rewrite endpoint allocation flow")
> >> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Thanks for the catch!

If you send v2 for the double quote fix in the commit message, you can
include this:

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

BR,
Thinh
Re: [PATCH] usb: dwc3: ep0: Don't reset resource alloc flag (including ep0)
Posted by Thinh Nguyen 1 year, 5 months ago 
On Wed, Aug 14, 2024, Thinh Nguyen wrote:
> Hi Michael,
> 
> On Wed, Aug 14, 2024, Sergey Shtylyov wrote:
> > On 8/14/24 11:42 PM, Sergey Shtylyov wrote:
> > [...]
> > 
> > >> The DWC3_EP_RESOURCE_ALLOCATED flag ensures that the resource of an
> > >> endpoint is only assigned once. Unless the endpoint is reset, don't
> > >> clear this flag. Otherwise we may set endpoint resource again, which
> > >> prevents the driver from initiate transfer after handling a STALL or
> > >> endpoint halt to the control endpoint.
> > >>
> > >> Commit f2e0eee47038 (usb: dwc3: ep0: Don't reset resource alloc flag)
> > > 
> > >    You forgot the double quotes around the summary, the same as you
> > > do in the Fixes tag.
> > > 
> > >> was fixing the initial issue, but did this only for physical ep1. Since
> > >> the function dwc3_ep0_stall_and_restart is resetting the flags for both
> > >> physical endpoints, this also has to be done for ep0.
> > >>
> > >> Cc: stable@vger.kernel.org
> > >> Fixes: b311048c174d ("usb: dwc3: gadget: Rewrite endpoint allocation flow")
> > >> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> 
> Thanks for the catch!
> 
> If you send v2 for the double quote fix in the commit message, you can
> include this:
> 
> Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> 

Actually, please ignore the Ack. Please do this instead:

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index d96ffbe52039..9b069c4663a1 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -232,7 +232,7 @@ void dwc3_ep0_stall_and_restart(struct dwc3 *dwc)
        /* stall is always issued on EP0 */
        dep = dwc->eps[0];
        __dwc3_gadget_ep_set_halt(dep, 1, false);
-       dep->flags = DWC3_EP_ENABLED;
+       dep->flags &= ~DWC3_EP_STALL;
        dwc->delayed_status = false;

        if (!list_empty(&dep->pending_list)) {


We don't want to clear other flags such as wedge flag.

BR,
Thinh
Re: [PATCH] usb: dwc3: ep0: Don't reset resource alloc flag (including ep0)
Posted by Thinh Nguyen 1 year, 5 months ago 
On Thu, Aug 15, 2024, Thinh Nguyen wrote:
> On Wed, Aug 14, 2024, Thinh Nguyen wrote:
> > Hi Michael,
> > 
> > On Wed, Aug 14, 2024, Sergey Shtylyov wrote:
> > > On 8/14/24 11:42 PM, Sergey Shtylyov wrote:
> > > [...]
> > > 
> > > >> The DWC3_EP_RESOURCE_ALLOCATED flag ensures that the resource of an
> > > >> endpoint is only assigned once. Unless the endpoint is reset, don't
> > > >> clear this flag. Otherwise we may set endpoint resource again, which
> > > >> prevents the driver from initiate transfer after handling a STALL or
> > > >> endpoint halt to the control endpoint.
> > > >>
> > > >> Commit f2e0eee47038 (usb: dwc3: ep0: Don't reset resource alloc flag)
> > > > 
> > > >    You forgot the double quotes around the summary, the same as you
> > > > do in the Fixes tag.
> > > > 
> > > >> was fixing the initial issue, but did this only for physical ep1. Since
> > > >> the function dwc3_ep0_stall_and_restart is resetting the flags for both
> > > >> physical endpoints, this also has to be done for ep0.
> > > >>
> > > >> Cc: stable@vger.kernel.org
> > > >> Fixes: b311048c174d ("usb: dwc3: gadget: Rewrite endpoint allocation flow")
> > > >> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> > 
> > Thanks for the catch!
> > 
> > If you send v2 for the double quote fix in the commit message, you can
> > include this:
> > 
> > Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> > 
> 
> Actually, please ignore the Ack. Please do this instead:
> 
> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index d96ffbe52039..9b069c4663a1 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
> @@ -232,7 +232,7 @@ void dwc3_ep0_stall_and_restart(struct dwc3 *dwc)
>         /* stall is always issued on EP0 */
>         dep = dwc->eps[0];
>         __dwc3_gadget_ep_set_halt(dep, 1, false);
> -       dep->flags = DWC3_EP_ENABLED;
> +       dep->flags &= ~DWC3_EP_STALL;
>         dwc->delayed_status = false;
> 
>         if (!list_empty(&dep->pending_list)) {
> 
> 
> We don't want to clear other flags such as wedge flag.
> 

Ugh... sorry for the spam... ignore the above. I forgot that we can't
wedge control ep. What you have is fine.

Thanks,
Thinh

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.