Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752988AbbEIAmY (ORCPT ); Fri, 8 May 2015 20:42:24 -0400 Received: from mga01.intel.com ([192.55.52.88]:17362 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751361AbbEIAmX (ORCPT ); Fri, 8 May 2015 20:42:23 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,395,1427785200"; d="scan'208";a="692215103" Message-ID: <554D57D8.9010207@linux.intel.com> Date: Sat, 09 May 2015 08:42:00 +0800 From: "Lu, Baolu" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Alan Stern CC: Greg Kroah-Hartman , Mathias Nyman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/3] usb: notify hcd when USB device suspend or resume References: In-Reply-To: Content-Type: text/plain; charset=windows-1252; 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: 4685 Lines: 111 On 05/08/2015 10:21 PM, Alan Stern wrote: > On Fri, 8 May 2015, Lu, Baolu wrote: > >> On 05/07/2015 10:34 PM, Alan Stern wrote: >>> On Thu, 7 May 2015, Lu, Baolu wrote: >>> >>>>>> + void (*device_suspend)(struct usb_hcd *, struct usb_device *udev, >>>>>> + pm_message_t msg); >>>>>> + void (*device_resume)(struct usb_hcd *, struct usb_device *udev, >>>>>> + pm_message_t msg); >>>>>> }; >>>>> Your callbacks don't use the msg argument. What makes you think it is >>>>> needed? >>>> This msg argument is valuable. XHCI spec defines a capability named FSC >>>> (Force Save context Capability). When this capability is implemented, the >>>> Save State operation (do during host suspend) shall save any cached Slot, >>>> Endpoint, Stream or other Context information to memory. xHCI hcd could >>>> use this "msg" to determine whether it needs to issue stop endpoint with >>>> SP (suspend) bit set. >>> I don't understand. What is the advantage of using FSC? >> I'm sorry, I didn't make it clear. >> >> As part of host suspend, controller save state will be issued to save >> host internal state in xhci_suspend(): > ... > >> If FSC is supported, the cached Slot, Endpoint, Stream, or other >> Context information are also saved. >> >> Hence, when FSC is supported, software does not have to issue Stop >> Endpoint Command to push public and private endpoint state into >> memory as part of system suspend process. > Why do you have to push this state into memory at all? Does the > controller hardware lose the cached state information when it is in low > power? I don't think controller hardware will lose the cached state information when it is in low power. But since cache in controller consumes power and resources, by pushing state into memory, hardware can power off the cache logic during suspend. Hence more power saving gains. > >> The logic in xhci_device_suspend() will look like: >> >> if xhci_device_suspend() callback was called due to system suspend, >> (mesg.event & PM_EVENT_SUSPEND is true) and FSC is supported by >> the xHC implementation, xhci_device_suspend() could ignore stop >> endpoint command, since CSS will be done in xhc_suspend() later and >> where all the endpoint caches will be pushed to memory. > I still don't understand this. You said earlier that according > to section 4.15.1.1 of the xHCI spec, the endpoint rings should > _always_ be stopped with SP set when a device is suspended. Now you're The intention of stop endpoint with SP set is to tell hardware that "a device is going to suspend, hardware don't need to contain the endpoint state in internal cache anymore". Hardware _could_ use this hint to push endpoint state into memory to reduce power consumption. > saying that they don't need to be stopped during a system suspend if > the controller supports FSC. (Or maybe you're saying they need to be > stopped but SP doesn't need to be set -- it's hard to tell.) Even FSC is supported, controller hardware still need to push cached endpoint state into memory when a USB device is suspended. The difference is when FSC is enforced, CSS command will push any cached endpoint state into memory unconditionally. So, when xhci_device_suspend() knows that CSS command will be executed later and CSS command will push cached endpoint state into memory (a.k.a. FSC is enforced), it could skip issuing stop endpoint command with SP bit set to avoid duplication and reduce the suspend time. This is the case for system suspend since CSS command is part of xhci_suspend() and xhci_suspend() will be executed after all USB devices have been suspended. But it's not case for run-time suspend (auto-pm) since USB device suspend and host controller suspend are independent for run-time case. That's the reason why I wanted to keep 'msg' parameter. But just as Greg said, we don't need to keep a parameter when it's not used and can add it later when it is required. > > So which is it? Do you need to stop the endpoint rings? Is it okay > not to set SP? "stop endpoint" and "stop endpoint with SP set" serve different purposes in Linux xhci driver as my understanding. "stop endpoint" command is used to stop a active ring when upper layer want to cancel a URB. "stop endpoint with SP set" is used to hint hardware that a USB is going to suspend. Hence "stop endpoint with SP set" must be executed in case that the transfer ring is empty. > > Alan Stern Thank you, Baolu > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/