Re: [PATCH] usb: cdnsp: Fix issue with Clear Feature Halt Endpoint

On 22-10-26 05:37:10, Pawel Laszczak wrote:

During handling Clear Halt Endpoint Feature request driver invokes Reset Endpoint command. Because this command has some issue with

What are issues? Would you please explain more?

transition endpoint from Running to Idle state the driver must stop the endpoint by using Stop Endpoint command.

cc: stable@vger.kernel.org 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 | 12 ++++-------- drivers/usb/cdns3/cdnsp-ring.c | 3 ++- 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c index e2e7d16f43f4..0576f9b0e4aa 100644 --- a/drivers/usb/cdns3/cdnsp-gadget.c +++ b/drivers/usb/cdns3/cdnsp-gadget.c @@ -600,11 +600,11 @@ int cdnsp_halt_endpoint(struct cdnsp_device *pdev, trace_cdnsp_ep_halt(value ? "Set" : "Clear");

• if (value) {

• ret = cdnsp_cmd_stop_ep(pdev, pep);
  

• if (ret)
  

• 	return ret;
  

• ret = cdnsp_cmd_stop_ep(pdev, pep);
• if (ret)

• return ret;
  

In your change ,it call cdnsp_cmd_stop_ep unconditionally, no matter set or clear halt? Is it your expectation? If it is, why?

• if (value) { if (GET_EP_CTX_STATE(pep->out_ctx) == EP_STATE_STOPPED) { cdnsp_queue_halt_endpoint(pdev, pep->idx); cdnsp_ring_cmd_db(pdev);

@@ -613,10 +613,6 @@ int cdnsp_halt_endpoint(struct cdnsp_device *pdev, pep->ep_state |= EP_HALTED; } else {

• /*
  

•  * In device mode driver can call reset endpoint command
  

•  * from any endpoint state.
  

•  */
  

  cdnsp_queue_reset_ep(pdev, pep->idx); cdnsp_ring_cmd_db(pdev); ret = cdnsp_wait_for_cmd_compl(pdev);

diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c index 25e5e51cf5a2..aa79bce89d8a 100644 --- a/drivers/usb/cdns3/cdnsp-ring.c +++ b/drivers/usb/cdns3/cdnsp-ring.c @@ -2081,7 +2081,8 @@ int cdnsp_cmd_stop_ep(struct cdnsp_device *pdev, struct cdnsp_ep *pep) u32 ep_state = GET_EP_CTX_STATE(pep->out_ctx); int ret = 0;

• if (ep_state == EP_STATE_STOPPED || ep_state == EP_STATE_DISABLED) {

• if (ep_state == EP_STATE_STOPPED || ep_state == EP_STATE_DISABLED ||

•    ep_state == EP_STATE_HALTED) {
  

  trace_cdnsp_ep_stopped_or_disabled(pep->out_ctx); goto ep_stopped; }

-- 2.25.1