Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932154AbdHWOav (ORCPT ); Wed, 23 Aug 2017 10:30:51 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:56136 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S932080AbdHWOau (ORCPT ); Wed, 23 Aug 2017 10:30:50 -0400 Date: Wed, 23 Aug 2017 10:30:47 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Mathias Nyman cc: anshuman.gupta@intel.com, , , , Subject: Re: [PATCH] usb: reducing an usb-port auto-resume latency. In-Reply-To: <599D3530.40603@linux.intel.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2903 Lines: 69 On Wed, 23 Aug 2017, Mathias Nyman wrote: > On 23.08.2017 09:18, anshuman.gupta@intel.com wrote: > > From: Anshuman Gupta > > > > This patch will improve the variable auto-resume latency of an usb-port. > > > > When xhci gets a port status change event interrupt due to PORT_PLC > > (port link state transition), linux Host controller driver drives the > > resume signalling on the bus for the amount of time defined by > > USB_REUME_TIMEOUT(40ms) macro. > > > > This 40ms delay for resume signalling is in acceptable limit, but > > it get worse when xhci goes for polling mode in order to detect other > > events on its ports and modify rh_timer timer with a variable time out of > > 1ms to (HZ/4)ms. > > > > drivers/usb/core/hcd.c line 799 > > mod_timer (&hcd->rh_timer, (jiffies/(HZ/4) + 1) * (HZ/4)). > > > > Due to above variable timeout usb auto-resume latency varies from > > 40ms to ~300ms. > > > > Log Snippet: > > ~128ms latency > > [ 53.112049] hub 1-0:1.0: state 7 ports 12 chg 0000 evt 0000 > > [ 53.229200] hub 1-0:1.0: state 7 ports 12 chg 0000 evt 0004 > > [ 53.240177] usb 1-2: usb wakeup-resume > > [ 53.240195] usb 1-2: finish resume > > [ 53.240357] usb usb1-port2: resume, status 0 > > ----------------------------------------------------------------- > > ~300ms latency > > [ 59.946620] hub 1-0:1.0: state 7 ports 12 chg 0000 evt 0000 > > [ 59.979341] hub 1-0:1.0: state 7 ports 12 chg 0000 evt 0000 > > [ 60.229342] hub 1-0:1.0: state 7 ports 12 chg 0000 evt 0004 > > [ 60.251321] usb 1-2: usb wakeup-resume > > [ 60.251335] usb 1-2: finish resume > > [ 60.251539] usb usb1-port2: resume, status 0 > > > > This variable resume latency can be optimized, as in case of PORT_PLC > > change event rh_timer has already been modified with USB_RESUME_TIMEOUT > > (40ms) delay,leaving the rest to GetPortStatus and started polling for > > root hub status (invoking usb_hcd_poll_rh_status). > > We can avoid polling as we have already modified rh_timer with > > delay of 40ms. > > > > This patch set the HCD_FLAG_POLL_RH to hcd->flags after modification of > > rh_timer, and avoids polling of root hub status. so rh_timer can fire > > after 40ms and usb device auto-resuem latency will be around 40ms. > > > > Signed-off-by: Anshuman Gupta > > --- > > Thanks, > adding and sending forward after 4.14-rc1 This patch description doesn't make sense. If you want to avoid resetting the root-hub timer by 250 ms, you should _clear_ the HCD_FLAG_POLL_RH flag, not _set_ it! See this (slightly tricky) code in usb_hcd_poll_rh_status(): if (hcd->uses_new_polling ? HCD_POLL_RH(hcd) : (length == 0 && hcd->status_urb != NULL)) mod_timer (&hcd->rh_timer, (jiffies/(HZ/4) + 1) * (HZ/4)); Since xhci-hcd does set the uses_new_polling flag, the "if" condition just tests the HCD_FLAG_POLL_RH bit. Alan Stern