diff mbox series

usb: cdnsp: Fix incorrect usb_request status

Message ID PH7PR07MB95382F640BC61712E986895BDD9D2@PH7PR07MB9538.namprd07.prod.outlook.com (mailing list archive)
State Superseded
Headers show
Series usb: cdnsp: Fix incorrect usb_request status | expand

Commit Message

Pawel Laszczak Sept. 5, 2024, 7:31 a.m. UTC
Fix changes incorrect usb_request->status returned during disabling
endpoints. Before fix the status returned during dequeuing requests
while disabling endpoint was ECONNRESET.
Patch changes it to ESHUTDOWN.

Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
cc: stable@vger.kernel.org
Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
 drivers/usb/cdns3/cdnsp-ring.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Peter Chen Sept. 5, 2024, 8:05 a.m. UTC | #1
On 24-09-05 07:31:10, Pawel Laszczak wrote:
> Fix changes incorrect usb_request->status returned during disabling
> endpoints. Before fix the status returned during dequeuing requests
> while disabling endpoint was ECONNRESET.
> Patch changes it to ESHUTDOWN.

Would you please explain why we need this change?

Peter

> 
> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> cc: stable@vger.kernel.org
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
>  drivers/usb/cdns3/cdnsp-ring.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
> index 1e011560e3ae..bccc8fc143d0 100644
> --- a/drivers/usb/cdns3/cdnsp-ring.c
> +++ b/drivers/usb/cdns3/cdnsp-ring.c
> @@ -718,7 +718,8 @@ int cdnsp_remove_request(struct cdnsp_device *pdev,
>  	seg = cdnsp_trb_in_td(pdev, cur_td->start_seg, cur_td->first_trb,
>  			      cur_td->last_trb, hw_deq);
>  
> -	if (seg && (pep->ep_state & EP_ENABLED))
> +	if (seg && (pep->ep_state & EP_ENABLED) &&
> +	    !(pep->ep_state & EP_DIS_IN_RROGRESS))
>  		cdnsp_find_new_dequeue_state(pdev, pep, preq->request.stream_id,
>  					     cur_td, &deq_state);
>  	else
> @@ -736,7 +737,8 @@ int cdnsp_remove_request(struct cdnsp_device *pdev,
>  	 * During disconnecting all endpoint will be disabled so we don't
>  	 * have to worry about updating dequeue pointer.
>  	 */
> -	if (pdev->cdnsp_state & CDNSP_STATE_DISCONNECT_PENDING) {
> +	if (pdev->cdnsp_state & CDNSP_STATE_DISCONNECT_PENDING ||
> +	    pep->ep_state & EP_DIS_IN_RROGRESS) {
>  		status = -ESHUTDOWN;
>  		ret = cdnsp_cmd_set_deq(pdev, pep, &deq_state);
>  	}
> -- 
> 2.43.0
>
Pawel Laszczak Sept. 5, 2024, 8:45 a.m. UTC | #2
>
>On 24-09-05 07:31:10, Pawel Laszczak wrote:
>> Fix changes incorrect usb_request->status returned during disabling
>> endpoints. Before fix the status returned during dequeuing requests
>> while disabling endpoint was ECONNRESET.
>> Patch changes it to ESHUTDOWN.
>
>Would you please explain why we need this change?

This patch is needed for UVC gadget. 
During stopping streaming the class starts dequeuing usb requests and
controller driver returns the -ECONNRESET status. After completion
requests the class or application "uvc-gadget" try to queue this
request again. Changing this status to ESHUTDOWN cause that UVC
assume that endpoint is disabled, or device is disconnected and
stop re-queuing usb requests.

Thanks,
Pawel

>
>Peter
>
>>
>> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence
>> USBSSP DRD Driver")
>> cc: stable@vger.kernel.org
>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>> ---
>>  drivers/usb/cdns3/cdnsp-ring.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/cdns3/cdnsp-ring.c
>> b/drivers/usb/cdns3/cdnsp-ring.c index 1e011560e3ae..bccc8fc143d0
>> 100644
>> --- a/drivers/usb/cdns3/cdnsp-ring.c
>> +++ b/drivers/usb/cdns3/cdnsp-ring.c
>> @@ -718,7 +718,8 @@ int cdnsp_remove_request(struct cdnsp_device
>*pdev,
>>  	seg = cdnsp_trb_in_td(pdev, cur_td->start_seg, cur_td->first_trb,
>>  			      cur_td->last_trb, hw_deq);
>>
>> -	if (seg && (pep->ep_state & EP_ENABLED))
>> +	if (seg && (pep->ep_state & EP_ENABLED) &&
>> +	    !(pep->ep_state & EP_DIS_IN_RROGRESS))
>>  		cdnsp_find_new_dequeue_state(pdev, pep, preq-
>>request.stream_id,
>>  					     cur_td, &deq_state);
>>  	else
>> @@ -736,7 +737,8 @@ int cdnsp_remove_request(struct cdnsp_device
>*pdev,
>>  	 * During disconnecting all endpoint will be disabled so we don't
>>  	 * have to worry about updating dequeue pointer.
>>  	 */
>> -	if (pdev->cdnsp_state & CDNSP_STATE_DISCONNECT_PENDING) {
>> +	if (pdev->cdnsp_state & CDNSP_STATE_DISCONNECT_PENDING ||
>> +	    pep->ep_state & EP_DIS_IN_RROGRESS) {
>>  		status = -ESHUTDOWN;
>>  		ret = cdnsp_cmd_set_deq(pdev, pep, &deq_state);
>>  	}
>> --
>> 2.43.0
>>
Peter Chen Sept. 6, 2024, 1:12 a.m. UTC | #3
On 24-09-05 08:45:59, Pawel Laszczak wrote:
> >
> >On 24-09-05 07:31:10, Pawel Laszczak wrote:
> >> Fix changes incorrect usb_request->status returned during disabling
> >> endpoints. Before fix the status returned during dequeuing requests
> >> while disabling endpoint was ECONNRESET.
> >> Patch changes it to ESHUTDOWN.
> >
> >Would you please explain why we need this change?
> 
> This patch is needed for UVC gadget. 
> During stopping streaming the class starts dequeuing usb requests and
> controller driver returns the -ECONNRESET status. After completion
> requests the class or application "uvc-gadget" try to queue this
> request again. Changing this status to ESHUTDOWN cause that UVC
> assume that endpoint is disabled, or device is disconnected and
> stop re-queuing usb requests.
> 

Get it. Would you please update commit message with your above
explanation?

Peter

> >
> >>
> >> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence
> >> USBSSP DRD Driver")
> >> cc: stable@vger.kernel.org
> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> >> ---
> >>  drivers/usb/cdns3/cdnsp-ring.c | 6 ++++--
> >>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/usb/cdns3/cdnsp-ring.c
> >> b/drivers/usb/cdns3/cdnsp-ring.c index 1e011560e3ae..bccc8fc143d0
> >> 100644
> >> --- a/drivers/usb/cdns3/cdnsp-ring.c
> >> +++ b/drivers/usb/cdns3/cdnsp-ring.c
> >> @@ -718,7 +718,8 @@ int cdnsp_remove_request(struct cdnsp_device
> >*pdev,
> >>  	seg = cdnsp_trb_in_td(pdev, cur_td->start_seg, cur_td->first_trb,
> >>  			      cur_td->last_trb, hw_deq);
> >>
> >> -	if (seg && (pep->ep_state & EP_ENABLED))
> >> +	if (seg && (pep->ep_state & EP_ENABLED) &&
> >> +	    !(pep->ep_state & EP_DIS_IN_RROGRESS))
> >>  		cdnsp_find_new_dequeue_state(pdev, pep, preq-
> >>request.stream_id,
> >>  					     cur_td, &deq_state);
> >>  	else
> >> @@ -736,7 +737,8 @@ int cdnsp_remove_request(struct cdnsp_device
> >*pdev,
> >>  	 * During disconnecting all endpoint will be disabled so we don't
> >>  	 * have to worry about updating dequeue pointer.
> >>  	 */
> >> -	if (pdev->cdnsp_state & CDNSP_STATE_DISCONNECT_PENDING) {
> >> +	if (pdev->cdnsp_state & CDNSP_STATE_DISCONNECT_PENDING ||
> >> +	    pep->ep_state & EP_DIS_IN_RROGRESS) {
> >>  		status = -ESHUTDOWN;
> >>  		ret = cdnsp_cmd_set_deq(pdev, pep, &deq_state);
> >>  	}
> >> --
> >> 2.43.0
> >>
diff mbox series

Patch

diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
index 1e011560e3ae..bccc8fc143d0 100644
--- a/drivers/usb/cdns3/cdnsp-ring.c
+++ b/drivers/usb/cdns3/cdnsp-ring.c
@@ -718,7 +718,8 @@  int cdnsp_remove_request(struct cdnsp_device *pdev,
 	seg = cdnsp_trb_in_td(pdev, cur_td->start_seg, cur_td->first_trb,
 			      cur_td->last_trb, hw_deq);
 
-	if (seg && (pep->ep_state & EP_ENABLED))
+	if (seg && (pep->ep_state & EP_ENABLED) &&
+	    !(pep->ep_state & EP_DIS_IN_RROGRESS))
 		cdnsp_find_new_dequeue_state(pdev, pep, preq->request.stream_id,
 					     cur_td, &deq_state);
 	else
@@ -736,7 +737,8 @@  int cdnsp_remove_request(struct cdnsp_device *pdev,
 	 * During disconnecting all endpoint will be disabled so we don't
 	 * have to worry about updating dequeue pointer.
 	 */
-	if (pdev->cdnsp_state & CDNSP_STATE_DISCONNECT_PENDING) {
+	if (pdev->cdnsp_state & CDNSP_STATE_DISCONNECT_PENDING ||
+	    pep->ep_state & EP_DIS_IN_RROGRESS) {
 		status = -ESHUTDOWN;
 		ret = cdnsp_cmd_set_deq(pdev, pep, &deq_state);
 	}