Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756064AbcLNO1u (ORCPT ); Wed, 14 Dec 2016 09:27:50 -0500 Received: from mga07.intel.com ([134.134.136.100]:12814 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755490AbcLNO1s (ORCPT ); Wed, 14 Dec 2016 09:27:48 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,346,1477983600"; d="scan'208";a="1081743066" Subject: Re: xhci_reset_endpoint() doesn't reset endpoint To: Michal Necasek , mathias.nyman@intel.com References: <7baa1154-5c0e-4c07-88e7-3e532404b69a@default> Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Dennis Wassenberg From: Mathias Nyman Message-ID: <58515718.50309@linux.intel.com> Date: Wed, 14 Dec 2016 16:28:40 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <7baa1154-5c0e-4c07-88e7-3e532404b69a@default> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2748 Lines: 58 On 14.12.2016 12:58, Michal Necasek wrote: > prior to the endpoint reset. SetFeature(CLEAR_HALT) resets the toggle > on the device, but not on the host. But we know for a fact that the > device sends a packet (with data toggle 0) which the host USB stack > never sees, and a data toggle mismatch explains that quite well. > > We are using USBFS to talk to the printer, but that shouldn't matter > much. I will note that the available documentation<1> explicitly says > that USBDEVFS_RESETEP and USBDEVFS_CLEAR_HALT both reset the data > toggle. That is indeed the case for the Linux EHCI driver but not > xHCI. Both of the USBFS IOCTLs call into xhci_reset_endpoint() which > does nothing. > This is very likely the case. xhci can not reset the host side of the endpoint unless it really is halted. xhci 4.6.8: "If the endpoint is not in the Halted state when an Reset Endpoint Command is executed -The xHC shall reject the command and generate a Command Completion Event with the Completion Code set to Context State Error." Normal halt/stall case is that xhci receives a STALL from the device, and immediately resets the endpoint (clears toggle, host side) then propagates the HALT status to usb core. USB core then sends SetFeature(CLEAR_HALT) to the device which will reset the toggle for the device side of the endpoint, and host and device toggles will be in sync. After this xhci_endpoint_reset() is called by usb core to inform xhci that the endpoint was reset, but currently we don't do anything in it. If SetFeature(CLEAR_HALT) is called without endpoint actually being HALTED we can not reset it from xhci. we should issue a config endpoint command to reset the host side toggle, as mentioned in xhci 1.0 120814 as a last note: "Note: The Reset Endpoint Command may only be issued to endpoints in the Halted state. If software wishes reset the Data Toggle or Sequence Number of an endpoint that isn't in the Halted state, then software may issue a Configure Endpoint Command with the Drop and Add bits set for the target endpoint. that is in the Stopped state." There was a case with a scanner we believed had the same issue, and we tried to resolve it by issuing the configure endpoint command in xhci_endpoint_reset() but if I remember correctly It did not resolve the case and code never got anywhere. I might have some really old implementation somewhere for this, at least there is a really old and outdated hack at git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git ep_reset_halt_test https://git.kernel.org/cgit/linux/kernel/git/mnyman/xhci.git/log/?h=ep_reset_halt_test which really is quite a hack, and based on 3.19 kernel so it's probably only useful as an Idea to base a real solution on. -Mathias