linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: cdnsp: Fixes issue with Configure Endpoint command
@ 2021-03-22  6:09 Pawel Laszczak
  2021-03-27  0:54 ` Peter Chen
  0 siblings, 1 reply; 4+ messages in thread
From: Pawel Laszczak @ 2021-03-22  6:09 UTC (permalink / raw)
  To: peter.chen; +Cc: gregkh, linux-usb, linux-kernel, kurahul, Pawel Laszczak

From: Pawel Laszczak <pawell@cadence.com>

Patch adds flag EP_UNCONFIGURED to detect whether endpoint was
unconfigured. This flag is set in cdnsp_reset_device after Reset Device
command. Among others this command disables all non control endpoints.
Flag is used in cdnsp_gadget_ep_disable to protect controller against
invoking Configure Endpoint command on disabled endpoint. Lack of this
protection in some cases caused that Configure Endpoint command completed
with Context State Error code completion.

Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
 drivers/usb/cdns3/cdnsp-gadget.c | 18 +++++++++++++-----
 drivers/usb/cdns3/cdnsp-gadget.h | 11 ++++++-----
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
index d7d4bdd57f46..de17cc4ad91a 100644
--- a/drivers/usb/cdns3/cdnsp-gadget.c
+++ b/drivers/usb/cdns3/cdnsp-gadget.c
@@ -727,7 +727,7 @@ int cdnsp_reset_device(struct cdnsp_device *pdev)
 	 * are in Disabled state.
 	 */
 	for (i = 1; i < CDNSP_ENDPOINTS_NUM; ++i)
-		pdev->eps[i].ep_state |= EP_STOPPED;
+		pdev->eps[i].ep_state |= EP_STOPPED | EP_UNCONFIGURED;
 
 	trace_cdnsp_handle_cmd_reset_dev(slot_ctx);
 
@@ -942,6 +942,7 @@ static int cdnsp_gadget_ep_enable(struct usb_ep *ep,
 
 	pep = to_cdnsp_ep(ep);
 	pdev = pep->pdev;
+	pep->ep_state &= ~EP_UNCONFIGURED;
 
 	if (dev_WARN_ONCE(pdev->dev, pep->ep_state & EP_ENABLED,
 			  "%s is already enabled\n", pep->name))
@@ -1023,9 +1024,13 @@ static int cdnsp_gadget_ep_disable(struct usb_ep *ep)
 		goto finish;
 	}
 
-	cdnsp_cmd_stop_ep(pdev, pep);
 	pep->ep_state |= EP_DIS_IN_RROGRESS;
-	cdnsp_cmd_flush_ep(pdev, pep);
+
+	/* Endpoint was unconfigured by Reset Device command. */
+	if (!(pep->ep_state & EP_UNCONFIGURED)) {
+		cdnsp_cmd_stop_ep(pdev, pep);
+		cdnsp_cmd_flush_ep(pdev, pep);
+	}
 
 	/* Remove all queued USB requests. */
 	while (!list_empty(&pep->pending_list)) {
@@ -1036,6 +1041,7 @@ static int cdnsp_gadget_ep_disable(struct usb_ep *ep)
 	cdnsp_invalidate_ep_events(pdev, pep);
 
 	pep->ep_state &= ~EP_DIS_IN_RROGRESS;
+
 	drop_flag = cdnsp_get_endpoint_flag(pep->endpoint.desc);
 	ctrl_ctx = cdnsp_get_input_control_ctx(&pdev->in_ctx);
 	ctrl_ctx->drop_flags = cpu_to_le32(drop_flag);
@@ -1043,10 +1049,12 @@ static int cdnsp_gadget_ep_disable(struct usb_ep *ep)
 
 	cdnsp_endpoint_zero(pdev, pep);
 
-	ret = cdnsp_update_eps_configuration(pdev, pep);
+	if (!(pep->ep_state & EP_UNCONFIGURED))
+		ret = cdnsp_update_eps_configuration(pdev, pep);
+
 	cdnsp_free_endpoint_rings(pdev, pep);
 
-	pep->ep_state &= ~EP_ENABLED;
+	pep->ep_state &= ~(EP_ENABLED | EP_UNCONFIGURED);
 	pep->ep_state |= EP_STOPPED;
 
 finish:
diff --git a/drivers/usb/cdns3/cdnsp-gadget.h b/drivers/usb/cdns3/cdnsp-gadget.h
index 6bbb26548c04..e628bd539e23 100644
--- a/drivers/usb/cdns3/cdnsp-gadget.h
+++ b/drivers/usb/cdns3/cdnsp-gadget.h
@@ -830,11 +830,12 @@ struct cdnsp_ep {
 	unsigned int ep_state;
 #define EP_ENABLED		BIT(0)
 #define EP_DIS_IN_RROGRESS	BIT(1)
-#define EP_HALTED		BIT(2)
-#define EP_STOPPED		BIT(3)
-#define EP_WEDGE		BIT(4)
-#define EP0_HALTED_STATUS	BIT(5)
-#define EP_HAS_STREAMS		BIT(6)
+#define EP_UNCONFIGURED		BIT(2)
+#define EP_HALTED		BIT(3)
+#define EP_STOPPED		BIT(4)
+#define EP_WEDGE		BIT(5)
+#define EP0_HALTED_STATUS	BIT(6)
+#define EP_HAS_STREAMS		BIT(7)
 
 	bool skip;
 };
-- 
2.25.1


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

* Re: [PATCH] usb: cdnsp: Fixes issue with Configure Endpoint command
  2021-03-22  6:09 [PATCH] usb: cdnsp: Fixes issue with Configure Endpoint command Pawel Laszczak
@ 2021-03-27  0:54 ` Peter Chen
  2021-03-30  4:26   ` Pawel Laszczak
  0 siblings, 1 reply; 4+ messages in thread
From: Peter Chen @ 2021-03-27  0:54 UTC (permalink / raw)
  To: Pawel Laszczak; +Cc: gregkh, linux-usb, linux-kernel, kurahul

On 21-03-22 07:09:02, Pawel Laszczak wrote:
> From: Pawel Laszczak <pawell@cadence.com>
> 
> Patch adds flag EP_UNCONFIGURED to detect whether endpoint was
> unconfigured. This flag is set in cdnsp_reset_device after Reset Device
> command. Among others this command disables all non control endpoints.
> Flag is used in cdnsp_gadget_ep_disable to protect controller against
> invoking Configure Endpoint command on disabled endpoint. Lack of this
> protection in some cases caused that Configure Endpoint command completed
> with Context State Error code completion.
> 
> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
>  drivers/usb/cdns3/cdnsp-gadget.c | 18 +++++++++++++-----
>  drivers/usb/cdns3/cdnsp-gadget.h | 11 ++++++-----
>  2 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
> index d7d4bdd57f46..de17cc4ad91a 100644
> --- a/drivers/usb/cdns3/cdnsp-gadget.c
> +++ b/drivers/usb/cdns3/cdnsp-gadget.c
> @@ -727,7 +727,7 @@ int cdnsp_reset_device(struct cdnsp_device *pdev)
>  	 * are in Disabled state.
>  	 */
>  	for (i = 1; i < CDNSP_ENDPOINTS_NUM; ++i)
> -		pdev->eps[i].ep_state |= EP_STOPPED;
> +		pdev->eps[i].ep_state |= EP_STOPPED | EP_UNCONFIGURED;
>  
>  	trace_cdnsp_handle_cmd_reset_dev(slot_ctx);
>  
> @@ -942,6 +942,7 @@ static int cdnsp_gadget_ep_enable(struct usb_ep *ep,
>  
>  	pep = to_cdnsp_ep(ep);
>  	pdev = pep->pdev;
> +	pep->ep_state &= ~EP_UNCONFIGURED;
>  
>  	if (dev_WARN_ONCE(pdev->dev, pep->ep_state & EP_ENABLED,
>  			  "%s is already enabled\n", pep->name))
> @@ -1023,9 +1024,13 @@ static int cdnsp_gadget_ep_disable(struct usb_ep *ep)
>  		goto finish;
>  	}
>  
> -	cdnsp_cmd_stop_ep(pdev, pep);
>  	pep->ep_state |= EP_DIS_IN_RROGRESS;
> -	cdnsp_cmd_flush_ep(pdev, pep);
> +
> +	/* Endpoint was unconfigured by Reset Device command. */
> +	if (!(pep->ep_state & EP_UNCONFIGURED)) {
> +		cdnsp_cmd_stop_ep(pdev, pep);
> +		cdnsp_cmd_flush_ep(pdev, pep);
> +	}
>  
>  	/* Remove all queued USB requests. */
>  	while (!list_empty(&pep->pending_list)) {
> @@ -1036,6 +1041,7 @@ static int cdnsp_gadget_ep_disable(struct usb_ep *ep)
>  	cdnsp_invalidate_ep_events(pdev, pep);
>  
>  	pep->ep_state &= ~EP_DIS_IN_RROGRESS;
> +

Useless blank line

>  	drop_flag = cdnsp_get_endpoint_flag(pep->endpoint.desc);
>  	ctrl_ctx = cdnsp_get_input_control_ctx(&pdev->in_ctx);
>  	ctrl_ctx->drop_flags = cpu_to_le32(drop_flag);
> @@ -1043,10 +1049,12 @@ static int cdnsp_gadget_ep_disable(struct usb_ep *ep)
>  
>  	cdnsp_endpoint_zero(pdev, pep);
>  
> -	ret = cdnsp_update_eps_configuration(pdev, pep);
> +	if (!(pep->ep_state & EP_UNCONFIGURED))
> +		ret = cdnsp_update_eps_configuration(pdev, pep);
> +
>  	cdnsp_free_endpoint_rings(pdev, pep);
>  
> -	pep->ep_state &= ~EP_ENABLED;
> +	pep->ep_state &= ~(EP_ENABLED | EP_UNCONFIGURED);
>  	pep->ep_state |= EP_STOPPED;
>  
>  finish:
> diff --git a/drivers/usb/cdns3/cdnsp-gadget.h b/drivers/usb/cdns3/cdnsp-gadget.h
> index 6bbb26548c04..e628bd539e23 100644
> --- a/drivers/usb/cdns3/cdnsp-gadget.h
> +++ b/drivers/usb/cdns3/cdnsp-gadget.h
> @@ -830,11 +830,12 @@ struct cdnsp_ep {
>  	unsigned int ep_state;
>  #define EP_ENABLED		BIT(0)
>  #define EP_DIS_IN_RROGRESS	BIT(1)
> -#define EP_HALTED		BIT(2)
> -#define EP_STOPPED		BIT(3)
> -#define EP_WEDGE		BIT(4)
> -#define EP0_HALTED_STATUS	BIT(5)
> -#define EP_HAS_STREAMS		BIT(6)
> +#define EP_UNCONFIGURED		BIT(2)

Why add new flag as BIT(2), it causes many changes in this patch?

> +#define EP_HALTED		BIT(3)
> +#define EP_STOPPED		BIT(4)
> +#define EP_WEDGE		BIT(5)
> +#define EP0_HALTED_STATUS	BIT(6)
> +#define EP_HAS_STREAMS		BIT(7)
>  
>  	bool skip;
>  };
> -- 
> 2.25.1
> 

-- 

Thanks,
Peter Chen


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

* RE: [PATCH] usb: cdnsp: Fixes issue with Configure Endpoint command
  2021-03-27  0:54 ` Peter Chen
@ 2021-03-30  4:26   ` Pawel Laszczak
  2021-04-02 23:57     ` Peter Chen
  0 siblings, 1 reply; 4+ messages in thread
From: Pawel Laszczak @ 2021-03-30  4:26 UTC (permalink / raw)
  To: Peter Chen; +Cc: gregkh, linux-usb, linux-kernel, Rahul Kumar

Hi Peter,

>
>On 21-03-22 07:09:02, Pawel Laszczak wrote:
>> From: Pawel Laszczak <pawell@cadence.com>
>>
>> Patch adds flag EP_UNCONFIGURED to detect whether endpoint was
>> unconfigured. This flag is set in cdnsp_reset_device after Reset Device
>> command. Among others this command disables all non control endpoints.
>> Flag is used in cdnsp_gadget_ep_disable to protect controller against
>> invoking Configure Endpoint command on disabled endpoint. Lack of this
>> protection in some cases caused that Configure Endpoint command completed
>> with Context State Error code completion.
>>
>> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>> ---
>>  drivers/usb/cdns3/cdnsp-gadget.c | 18 +++++++++++++-----
>>  drivers/usb/cdns3/cdnsp-gadget.h | 11 ++++++-----
>>  2 files changed, 19 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
>> index d7d4bdd57f46..de17cc4ad91a 100644
>> --- a/drivers/usb/cdns3/cdnsp-gadget.c
>> +++ b/drivers/usb/cdns3/cdnsp-gadget.c
>> @@ -727,7 +727,7 @@ int cdnsp_reset_device(struct cdnsp_device *pdev)
>>  	 * are in Disabled state.
>>  	 */
>>  	for (i = 1; i < CDNSP_ENDPOINTS_NUM; ++i)
>> -		pdev->eps[i].ep_state |= EP_STOPPED;
>> +		pdev->eps[i].ep_state |= EP_STOPPED | EP_UNCONFIGURED;
>>
>>  	trace_cdnsp_handle_cmd_reset_dev(slot_ctx);
>>
>> @@ -942,6 +942,7 @@ static int cdnsp_gadget_ep_enable(struct usb_ep *ep,
>>
>>  	pep = to_cdnsp_ep(ep);
>>  	pdev = pep->pdev;
>> +	pep->ep_state &= ~EP_UNCONFIGURED;
>>
>>  	if (dev_WARN_ONCE(pdev->dev, pep->ep_state & EP_ENABLED,
>>  			  "%s is already enabled\n", pep->name))
>> @@ -1023,9 +1024,13 @@ static int cdnsp_gadget_ep_disable(struct usb_ep *ep)
>>  		goto finish;
>>  	}
>>
>> -	cdnsp_cmd_stop_ep(pdev, pep);
>>  	pep->ep_state |= EP_DIS_IN_RROGRESS;
>> -	cdnsp_cmd_flush_ep(pdev, pep);
>> +
>> +	/* Endpoint was unconfigured by Reset Device command. */
>> +	if (!(pep->ep_state & EP_UNCONFIGURED)) {
>> +		cdnsp_cmd_stop_ep(pdev, pep);
>> +		cdnsp_cmd_flush_ep(pdev, pep);
>> +	}
>>
>>  	/* Remove all queued USB requests. */
>>  	while (!list_empty(&pep->pending_list)) {
>> @@ -1036,6 +1041,7 @@ static int cdnsp_gadget_ep_disable(struct usb_ep *ep)
>>  	cdnsp_invalidate_ep_events(pdev, pep);
>>
>>  	pep->ep_state &= ~EP_DIS_IN_RROGRESS;
>> +
>
>Useless blank line
>
>>  	drop_flag = cdnsp_get_endpoint_flag(pep->endpoint.desc);
>>  	ctrl_ctx = cdnsp_get_input_control_ctx(&pdev->in_ctx);
>>  	ctrl_ctx->drop_flags = cpu_to_le32(drop_flag);
>> @@ -1043,10 +1049,12 @@ static int cdnsp_gadget_ep_disable(struct usb_ep *ep)
>>
>>  	cdnsp_endpoint_zero(pdev, pep);
>>
>> -	ret = cdnsp_update_eps_configuration(pdev, pep);
>> +	if (!(pep->ep_state & EP_UNCONFIGURED))
>> +		ret = cdnsp_update_eps_configuration(pdev, pep);
>> +
>>  	cdnsp_free_endpoint_rings(pdev, pep);
>>
>> -	pep->ep_state &= ~EP_ENABLED;
>> +	pep->ep_state &= ~(EP_ENABLED | EP_UNCONFIGURED);
>>  	pep->ep_state |= EP_STOPPED;
>>
>>  finish:
>> diff --git a/drivers/usb/cdns3/cdnsp-gadget.h b/drivers/usb/cdns3/cdnsp-gadget.h
>> index 6bbb26548c04..e628bd539e23 100644
>> --- a/drivers/usb/cdns3/cdnsp-gadget.h
>> +++ b/drivers/usb/cdns3/cdnsp-gadget.h
>> @@ -830,11 +830,12 @@ struct cdnsp_ep {
>>  	unsigned int ep_state;
>>  #define EP_ENABLED		BIT(0)
>>  #define EP_DIS_IN_RROGRESS	BIT(1)
>> -#define EP_HALTED		BIT(2)
>> -#define EP_STOPPED		BIT(3)
>> -#define EP_WEDGE		BIT(4)
>> -#define EP0_HALTED_STATUS	BIT(5)
>> -#define EP_HAS_STREAMS		BIT(6)
>> +#define EP_UNCONFIGURED		BIT(2)
>
>Why add new flag as BIT(2), it causes many changes in this patch?

In my feeling, EP_UNCONFIGURED is more associates with the first 2 flags, so I've decided 
put it after BIT(1).  

>
>> +#define EP_HALTED		BIT(3)
>> +#define EP_STOPPED		BIT(4)
>> +#define EP_WEDGE		BIT(5)
>> +#define EP0_HALTED_STATUS	BIT(6)
>> +#define EP_HAS_STREAMS		BIT(7)
>>
>>  	bool skip;
>>  };
>> --
>> 2.25.1
>>
>
>--
>
>Thanks,
>Peter Chen


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

* Re: [PATCH] usb: cdnsp: Fixes issue with Configure Endpoint command
  2021-03-30  4:26   ` Pawel Laszczak
@ 2021-04-02 23:57     ` Peter Chen
  0 siblings, 0 replies; 4+ messages in thread
From: Peter Chen @ 2021-04-02 23:57 UTC (permalink / raw)
  To: Pawel Laszczak; +Cc: gregkh, linux-usb, linux-kernel, Rahul Kumar

On 21-03-30 04:26:10, Pawel Laszczak wrote:
> Hi Peter,
> 
> >
> >On 21-03-22 07:09:02, Pawel Laszczak wrote:
> >> From: Pawel Laszczak <pawell@cadence.com>
> >>
> >> Patch adds flag EP_UNCONFIGURED to detect whether endpoint was
> >> unconfigured. This flag is set in cdnsp_reset_device after Reset Device
> >> command. Among others this command disables all non control endpoints.
> >> Flag is used in cdnsp_gadget_ep_disable to protect controller against
> >> invoking Configure Endpoint command on disabled endpoint. Lack of this
> >> protection in some cases caused that Configure Endpoint command completed
> >> with Context State Error code completion.
> >>
> >> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> >> ---
> >>  drivers/usb/cdns3/cdnsp-gadget.c | 18 +++++++++++++-----
> >>  drivers/usb/cdns3/cdnsp-gadget.h | 11 ++++++-----
> >>  2 files changed, 19 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
> >> index d7d4bdd57f46..de17cc4ad91a 100644
> >> --- a/drivers/usb/cdns3/cdnsp-gadget.c
> >> +++ b/drivers/usb/cdns3/cdnsp-gadget.c
> >> @@ -727,7 +727,7 @@ int cdnsp_reset_device(struct cdnsp_device *pdev)
> >>  	 * are in Disabled state.
> >>  	 */
> >>  	for (i = 1; i < CDNSP_ENDPOINTS_NUM; ++i)
> >> -		pdev->eps[i].ep_state |= EP_STOPPED;
> >> +		pdev->eps[i].ep_state |= EP_STOPPED | EP_UNCONFIGURED;
> >>
> >>  	trace_cdnsp_handle_cmd_reset_dev(slot_ctx);
> >>
> >> @@ -942,6 +942,7 @@ static int cdnsp_gadget_ep_enable(struct usb_ep *ep,
> >>
> >>  	pep = to_cdnsp_ep(ep);
> >>  	pdev = pep->pdev;
> >> +	pep->ep_state &= ~EP_UNCONFIGURED;
> >>
> >>  	if (dev_WARN_ONCE(pdev->dev, pep->ep_state & EP_ENABLED,
> >>  			  "%s is already enabled\n", pep->name))
> >> @@ -1023,9 +1024,13 @@ static int cdnsp_gadget_ep_disable(struct usb_ep *ep)
> >>  		goto finish;
> >>  	}
> >>
> >> -	cdnsp_cmd_stop_ep(pdev, pep);
> >>  	pep->ep_state |= EP_DIS_IN_RROGRESS;
> >> -	cdnsp_cmd_flush_ep(pdev, pep);
> >> +
> >> +	/* Endpoint was unconfigured by Reset Device command. */
> >> +	if (!(pep->ep_state & EP_UNCONFIGURED)) {
> >> +		cdnsp_cmd_stop_ep(pdev, pep);
> >> +		cdnsp_cmd_flush_ep(pdev, pep);
> >> +	}
> >>
> >>  	/* Remove all queued USB requests. */
> >>  	while (!list_empty(&pep->pending_list)) {
> >> @@ -1036,6 +1041,7 @@ static int cdnsp_gadget_ep_disable(struct usb_ep *ep)
> >>  	cdnsp_invalidate_ep_events(pdev, pep);
> >>
> >>  	pep->ep_state &= ~EP_DIS_IN_RROGRESS;
> >> +
> >
> >Useless blank line
> >
> >>  	drop_flag = cdnsp_get_endpoint_flag(pep->endpoint.desc);
> >>  	ctrl_ctx = cdnsp_get_input_control_ctx(&pdev->in_ctx);
> >>  	ctrl_ctx->drop_flags = cpu_to_le32(drop_flag);
> >> @@ -1043,10 +1049,12 @@ static int cdnsp_gadget_ep_disable(struct usb_ep *ep)
> >>
> >>  	cdnsp_endpoint_zero(pdev, pep);
> >>
> >> -	ret = cdnsp_update_eps_configuration(pdev, pep);
> >> +	if (!(pep->ep_state & EP_UNCONFIGURED))
> >> +		ret = cdnsp_update_eps_configuration(pdev, pep);
> >> +
> >>  	cdnsp_free_endpoint_rings(pdev, pep);
> >>
> >> -	pep->ep_state &= ~EP_ENABLED;
> >> +	pep->ep_state &= ~(EP_ENABLED | EP_UNCONFIGURED);
> >>  	pep->ep_state |= EP_STOPPED;
> >>
> >>  finish:
> >> diff --git a/drivers/usb/cdns3/cdnsp-gadget.h b/drivers/usb/cdns3/cdnsp-gadget.h
> >> index 6bbb26548c04..e628bd539e23 100644
> >> --- a/drivers/usb/cdns3/cdnsp-gadget.h
> >> +++ b/drivers/usb/cdns3/cdnsp-gadget.h
> >> @@ -830,11 +830,12 @@ struct cdnsp_ep {
> >>  	unsigned int ep_state;
> >>  #define EP_ENABLED		BIT(0)
> >>  #define EP_DIS_IN_RROGRESS	BIT(1)
> >> -#define EP_HALTED		BIT(2)
> >> -#define EP_STOPPED		BIT(3)
> >> -#define EP_WEDGE		BIT(4)
> >> -#define EP0_HALTED_STATUS	BIT(5)
> >> -#define EP_HAS_STREAMS		BIT(6)
> >> +#define EP_UNCONFIGURED		BIT(2)
> >
> >Why add new flag as BIT(2), it causes many changes in this patch?
> 
> In my feeling, EP_UNCONFIGURED is more associates with the first 2 flags, so I've decided 
> put it after BIT(1).  

No, you may not add such relationship, each flag has its own meaning,
otherwise, the sequence of flag bitmap may be changed again.

Peter

> 
> >
> >> +#define EP_HALTED		BIT(3)
> >> +#define EP_STOPPED		BIT(4)
> >> +#define EP_WEDGE		BIT(5)
> >> +#define EP0_HALTED_STATUS	BIT(6)
> >> +#define EP_HAS_STREAMS		BIT(7)
> >>
> >>  	bool skip;
> >>  };
> >> --
> >> 2.25.1
> >>
> >
> >--
> >
> >Thanks,
> >Peter Chen
> 

-- 

Thanks,
Peter Chen


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

end of thread, other threads:[~2021-04-02 23:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22  6:09 [PATCH] usb: cdnsp: Fixes issue with Configure Endpoint command Pawel Laszczak
2021-03-27  0:54 ` Peter Chen
2021-03-30  4:26   ` Pawel Laszczak
2021-04-02 23:57     ` Peter Chen

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).