2020-08-21 07:21:57

by Ding Hui

[permalink] [raw]
Subject: [PATCH] xhci: Always restore EP_SOFT_CLEAR_TOGGLE even if ep reset failed

Some devices driver call libusb_clear_halt when target ep queue
is not empty. (eg. spice client connected to qemu for usb redir)

Before commit f5249461b504 ("xhci: Clear the host side toggle
manually when endpoint is soft reset"), that works well.
But now, we got the error log:

EP not empty, refuse reset

xhch_endpoint_reset failed and left ep_state's EP_SOFT_CLEAR_TOGGLE
bit is still on

So all the subsequent urb sumbit to the ep will fail with the
warn log:

Can't enqueue URB while manually clearing toggle

We need restore ep_state EP_SOFT_CLEAR_TOGGLE bit after
xhci_endpoint_reset, even if it is failed.

Signed-off-by: Ding Hui <[email protected]>
---
drivers/usb/host/xhci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 3c41b14ecce7..e9884ae9c77d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3236,10 +3236,11 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,

wait_for_completion(cfg_cmd->completion);

- ep->ep_state &= ~EP_SOFT_CLEAR_TOGGLE;
xhci_free_command(xhci, cfg_cmd);
cleanup:
xhci_free_command(xhci, stop_cmd);
+ if (ep->ep_state & EP_SOFT_CLEAR_TOGGLE)
+ ep->ep_state &= ~EP_SOFT_CLEAR_TOGGLE;
}

static int xhci_check_streams_endpoint(struct xhci_hcd *xhci,
--
2.17.1


2020-08-21 07:34:36

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] xhci: Always restore EP_SOFT_CLEAR_TOGGLE even if ep reset failed

On Fri, Aug 21, 2020 at 03:06:52PM +0800, Ding Hui wrote:
> Some devices driver call libusb_clear_halt when target ep queue
> is not empty. (eg. spice client connected to qemu for usb redir)
>
> Before commit f5249461b504 ("xhci: Clear the host side toggle
> manually when endpoint is soft reset"), that works well.
> But now, we got the error log:
>
> EP not empty, refuse reset
>
> xhch_endpoint_reset failed and left ep_state's EP_SOFT_CLEAR_TOGGLE
> bit is still on
>
> So all the subsequent urb sumbit to the ep will fail with the
> warn log:
>
> Can't enqueue URB while manually clearing toggle
>
> We need restore ep_state EP_SOFT_CLEAR_TOGGLE bit after
> xhci_endpoint_reset, even if it is failed.
>
> Signed-off-by: Ding Hui <[email protected]>
> ---
> drivers/usb/host/xhci.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)

Shouldn't this have a Fixes: tag on it and be backported to the affected
stable trees?

thanks,

greg k-h

2020-08-21 08:42:59

by Mathias Nyman

[permalink] [raw]
Subject: Re: [PATCH] xhci: Always restore EP_SOFT_CLEAR_TOGGLE even if ep reset failed

On 21.8.2020 10.31, Greg KH wrote:
> On Fri, Aug 21, 2020 at 03:06:52PM +0800, Ding Hui wrote:
>> Some devices driver call libusb_clear_halt when target ep queue
>> is not empty. (eg. spice client connected to qemu for usb redir)
>>
>> Before commit f5249461b504 ("xhci: Clear the host side toggle
>> manually when endpoint is soft reset"), that works well.
>> But now, we got the error log:
>>
>> EP not empty, refuse reset
>>
>> xhch_endpoint_reset failed and left ep_state's EP_SOFT_CLEAR_TOGGLE
>> bit is still on
>>
>> So all the subsequent urb sumbit to the ep will fail with the
>> warn log:
>>
>> Can't enqueue URB while manually clearing toggle
>>
>> We need restore ep_state EP_SOFT_CLEAR_TOGGLE bit after
>> xhci_endpoint_reset, even if it is failed.
>>
>> Signed-off-by: Ding Hui <[email protected]>

Thanks, nice catch.

>> ---
>> drivers/usb/host/xhci.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Shouldn't this have a Fixes: tag on it and be backported to the affected
> stable trees?

It should, but I like this patch and want it in, so I'll add the tags this time.

Thanks
-Mathias

2020-08-22 00:00:31

by Ding Hui

[permalink] [raw]
Subject: Re: [PATCH] xhci: Always restore EP_SOFT_CLEAR_TOGGLE even if ep reset failed

On 2020/8/21 4:44 下午, Mathias Nyman wrote:
> On 21.8.2020 10.31, Greg KH wrote:
>> On Fri, Aug 21, 2020 at 03:06:52PM +0800, Ding Hui wrote:
>>> Some devices driver call libusb_clear_halt when target ep queue
>>> is not empty. (eg. spice client connected to qemu for usb redir)
>>>
>>> Before commit f5249461b504 ("xhci: Clear the host side toggle
>>> manually when endpoint is soft reset"), that works well.
>>> But now, we got the error log:
>>>
>>> EP not empty, refuse reset
>>>
>>> xhch_endpoint_reset failed and left ep_state's EP_SOFT_CLEAR_TOGGLE
>>> bit is still on
>>>
>>> So all the subsequent urb sumbit to the ep will fail with the
>>> warn log:
>>>
>>> Can't enqueue URB while manually clearing toggle
>>>
>>> We need restore ep_state EP_SOFT_CLEAR_TOGGLE bit after
>>> xhci_endpoint_reset, even if it is failed.
>>>
>>> Signed-off-by: Ding Hui <[email protected]>
>
> Thanks, nice catch.
>
>>> ---
>>> drivers/usb/host/xhci.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> Shouldn't this have a Fixes: tag on it and be backported to the affected
>> stable trees?

Sorry for missing the tags, this is my first kernel patch :-)

>
> It should, but I like this patch and want it in, so I'll add the tags this time.

Thanks for correcting my commit msg

>
> Thanks
> -Mathias
>


--
Thanks,
-dinghui