Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] xhci: Always restore EP_SOFT_CLEAR_TOGGLE even if ep
 reset failed
To:     Mathias Nyman <mathias.nyman@linux.intel.com>,
        Greg KH <gregkh@linuxfoundation.org>
Cc:     mathias.nyman@intel.com, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20200821070652.27782-1-dinghui@sangfor.com.cn>
 <20200821073147.GA1681156@kroah.com>
 <c6275105-a204-fe23-2dae-2bfa6c06a839@linux.intel.com>
From:   Ding Hui <dinghui@sangfor.com.cn>
Message-ID: <576e0013-a11e-4ffb-3964-cc9aba204245@sangfor.com.cn>
Date:   Sat, 22 Aug 2020 07:58:29 +0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0)
 Gecko/20100101 Thunderbird/78.1.1
MIME-Version: 1.0
In-Reply-To: <c6275105-a204-fe23-2dae-2bfa6c06a839@linux.intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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 <dinghui@sangfor.com.cn>
> 
> 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