From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932245AbcHIK0U (ORCPT ); Tue, 9 Aug 2016 06:26:20 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:55914 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751590AbcHIK0T (ORCPT ); Tue, 9 Aug 2016 06:26:19 -0400 Date: Tue, 9 Aug 2016 12:26:31 +0200 From: Greg KH To: Baolin Wang Cc: stefan.koch10@gmail.com, stern@rowland.harvard.edu, oneukum@suse.com, falakreyaz@gmail.com, broonie@kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] usb: core: Add runtime resume checking Message-ID: <20160809102631.GA30327@kroah.com> References: <653c8e893d4de1ab4dcade2ca2da9f2cfe369c2c.1470735102.git.baolin.wang@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <653c8e893d4de1ab4dcade2ca2da9f2cfe369c2c.1470735102.git.baolin.wang@linaro.org> User-Agent: Mutt/1.6.2 (2016-07-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Aug 09, 2016 at 05:33:33PM +0800, Baolin Wang wrote: > When the usb device has entered suspend state by runtime suspend method, and > the sustem also try to enter suspend state by issuing usb_dev_suspend(), it > will issue pm_runtime_resume() function to deal with wrong wakeup setting in > choose_wakeup() function. > > But if usb device resumes failed due to xhci has been into suspend state and > hardware is not accessible, which will set runtime errors. Thus when there is > slave attached, usb device will resume failed by runtime resume method due to > previous runtime errors. I really can't parse the first sentance in this paragraph, what exactly makes xhci so "unique" here? > Then we should check if it resumes successfully in choose_wakeup() function, what is "it"? > if it failed we should clear the runtime errors by pm_runtime_set_suspended() > function to avoid runtime resume failure. Again, what is "it"? > > Signed-off-by: Baolin Wang > --- > drivers/usb/core/driver.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c > index dadd1e8d..a1a0f5f 100644 > --- a/drivers/usb/core/driver.c > +++ b/drivers/usb/core/driver.c > @@ -1412,6 +1412,7 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg) > static void choose_wakeup(struct usb_device *udev, pm_message_t msg) > { > int w; > + int ret; > > /* Remote wakeup is needed only when we actually go to sleep. > * For things like FREEZE and QUIESCE, if the device is already > @@ -1431,8 +1432,12 @@ static void choose_wakeup(struct usb_device *udev, pm_message_t msg) > /* If the device is autosuspended with the wrong wakeup setting, > * autoresume now so the setting can be changed. > */ > - if (udev->state == USB_STATE_SUSPENDED && w != udev->do_remote_wakeup) > - pm_runtime_resume(&udev->dev); > + if (udev->state == USB_STATE_SUSPENDED && w != udev->do_remote_wakeup) { > + ret = pm_runtime_resume(&udev->dev); > + if (ret == -ESHUTDOWN) > + pm_runtime_set_suspended(&udev->dev); why is 'ret' needed: if (pm_runtime_resume(&udev->dev) == -ESHUTDOWN) Why would resume fail? thanks, greg k-h