Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965618AbbEFBHb (ORCPT ); Tue, 5 May 2015 21:07:31 -0400 Received: from mga03.intel.com ([134.134.136.65]:58763 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932150AbbEFBH1 (ORCPT ); Tue, 5 May 2015 21:07:27 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,376,1427785200"; d="scan'208";a="724338401" Message-ID: <5549694D.6000505@linux.intel.com> Date: Wed, 06 May 2015 09:07:25 +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: [RFC][PATCH 1/3] usb: add a hcd notify entry in hc_driver References: In-Reply-To: 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: 2412 Lines: 72 On 05/05/2015 10:50 PM, Alan Stern wrote: > On Tue, 5 May 2015, Lu, Baolu wrote: > >> The order that software should do during device suspend/resume is >> defined in 4.15.1.1 of xHCI spec 1.1. >> >> Spec 4.15.1.1: >> >> Software shall stop all endpoints of a device using the Stop Endpoint >> Command and setting the Suspend >> (SP) flag to 1 prior to selectively suspending a device. > But _after_ all the URBs sent to the device have completed, right? Yes, that's right. > >> After the >> device is resumed software shall ring an >> endpoint's doorbell to restart it. > The driver would ring the endpoint's doorbell anyway when a new URB is > submitted, wouldn't it? Which means the resume callback doesn't > actually have to do anything. The value of ringing door bell after resume is that hcd can fetch endpoint state from memory to cache and get ready for transfer as early as possible. > >> --end-- >> >> So the order looks like: >> >> tell hcd device suspend >> usb_port_suspend() > You have forgotten that usb_port_suspend() can send URBs to the device > (to enable remote wakeup, for example). Therefore you shouldn't notify > the HCD until usb_port_suspend() is partly or totally finished. Yes, I agree with you. I will move the notification into usb_port_suspend(), just before sending suspend request to parent hub. > >> usb_port_resume() >> tell hcd device resume > You have also forgotten that usb_port_resume() calls various functions > that send URBs to ep0 on the device. Therefore if the HCD's > device_resume callback needs to do something (like ringing ep0's > doorbell), you had better invoke the callback _before_ calling > usb_port_resume(). Or maybe you had better do this _within_ > usb_port_resume(). Agree. I will do this within usb_port_resume() just after completing sending clear port feature. > > Alan Stern Thank you for the comments. I will send the v2 patch series with all your comments addressed. Thanks, Baolu > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- 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/