RE: [PATCH v2] usb: cdnsp: Fixes issue with Configure Endpoint command
>
>On 21-04-07 08:36:29, Pawel Laszczak wrote:
>> From: Pawel Laszczak
>>
>> 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
>
>Pawel, it is a little late for v5.12, I apply it to v5.13-rc1 if you
>don't mind.
>
>Peter
Yes, you can apply it to v5.13-rc1.
Thanks.
r
>>
>> ---
>> Changelog:
>> v2:
>> - removed useless blank line
>> - changed the EP_UNCONFIGURED to limit changes in patch
>>
>> drivers/usb/cdns3/cdnsp-gadget.c | 17 -
>> drivers/usb/cdns3/cdnsp-gadget.h | 1 +
>> 2 files changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c
>> b/drivers/usb/cdns3/cdnsp-gadget.c
>> index d7d4bdd57f46..56707b6b0f57 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(>pending_list)) {
>> @@ -1043,10 +1048,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..783ca8ffde00 100644
>> --- a/drivers/usb/cdns3/cdnsp-gadget.h
>> +++ b/drivers/usb/cdns3/cdnsp-gadget.h
>> @@ -835,6 +835,7 @@ struct cdnsp_ep {
>> #define EP_WEDGEBIT(4)
>> #define EP0_HALTED_STATUS BIT(5)
>> #define EP_HAS_STREAMS BIT(6)
>> +#define EP_UNCONFIGURED BIT(7)
>>
>> bool skip;
>> };
>> --
>> 2.25.1
>>
>
>--
>
>Thanks,
>Peter Chen
--
Regards,
Pawel Laszczak
Re: [PATCH v2] usb: cdnsp: Fixes issue with Configure Endpoint command
On 21-04-07 08:36:29, Pawel Laszczak wrote:
> From: Pawel Laszczak
>
> 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
Pawel, it is a little late for v5.12, I apply it to v5.13-rc1 if you
don't mind.
Peter
>
> ---
> Changelog:
> v2:
> - removed useless blank line
> - changed the EP_UNCONFIGURED to limit changes in patch
>
> drivers/usb/cdns3/cdnsp-gadget.c | 17 -
> drivers/usb/cdns3/cdnsp-gadget.h | 1 +
> 2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c
> b/drivers/usb/cdns3/cdnsp-gadget.c
> index d7d4bdd57f46..56707b6b0f57 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(>pending_list)) {
> @@ -1043,10 +1048,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..783ca8ffde00 100644
> --- a/drivers/usb/cdns3/cdnsp-gadget.h
> +++ b/drivers/usb/cdns3/cdnsp-gadget.h
> @@ -835,6 +835,7 @@ struct cdnsp_ep {
> #define EP_WEDGE BIT(4)
> #define EP0_HALTED_STATUSBIT(5)
> #define EP_HAS_STREAMS BIT(6)
> +#define EP_UNCONFIGURED BIT(7)
>
> bool skip;
> };
> --
> 2.25.1
>
--
Thanks,
Peter Chen
Re: [PATCH v2] usb: cdnsp: Fixes issue with Configure Endpoint command
On 21-04-07 08:36:29, Pawel Laszczak wrote:
> From: Pawel Laszczak
>
> 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
Pawel, it is a little later for v5.12. I queue it to v5.13-rc1 if you
don't mind.
Peter
>
> ---
> Changelog:
> v2:
> - removed useless blank line
> - changed the EP_UNCONFIGURED to limit changes in patch
>
> drivers/usb/cdns3/cdnsp-gadget.c | 17 -
> drivers/usb/cdns3/cdnsp-gadget.h | 1 +
> 2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c
> b/drivers/usb/cdns3/cdnsp-gadget.c
> index d7d4bdd57f46..56707b6b0f57 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(>pending_list)) {
> @@ -1043,10 +1048,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..783ca8ffde00 100644
> --- a/drivers/usb/cdns3/cdnsp-gadget.h
> +++ b/drivers/usb/cdns3/cdnsp-gadget.h
> @@ -835,6 +835,7 @@ struct cdnsp_ep {
> #define EP_WEDGE BIT(4)
> #define EP0_HALTED_STATUSBIT(5)
> #define EP_HAS_STREAMS BIT(6)
> +#define EP_UNCONFIGURED BIT(7)
>
> bool skip;
> };
> --
> 2.25.1
>
--
Thanks,
Peter Chen
[PATCH v2] usb: cdnsp: Fixes issue with Configure Endpoint command
From: Pawel Laszczak
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
---
Changelog:
v2:
- removed useless blank line
- changed the EP_UNCONFIGURED to limit changes in patch
drivers/usb/cdns3/cdnsp-gadget.c | 17 -
drivers/usb/cdns3/cdnsp-gadget.h | 1 +
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
index d7d4bdd57f46..56707b6b0f57 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(>pending_list)) {
@@ -1043,10 +1048,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..783ca8ffde00 100644
--- a/drivers/usb/cdns3/cdnsp-gadget.h
+++ b/drivers/usb/cdns3/cdnsp-gadget.h
@@ -835,6 +835,7 @@ struct cdnsp_ep {
#define EP_WEDGE BIT(4)
#define EP0_HALTED_STATUS BIT(5)
#define EP_HAS_STREAMS BIT(6)
+#define EP_UNCONFIGUREDBIT(7)
bool skip;
};
--
2.25.1
