From patchwork Tue Nov 15 08:51:29 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lin Ming X-Patchwork-Id: 125707 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id E6073B6F76 for ; Tue, 15 Nov 2011 19:51:34 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754002Ab1KOIvd (ORCPT ); Tue, 15 Nov 2011 03:51:33 -0500 Received: from mga02.intel.com ([134.134.136.20]:12066 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753997Ab1KOIvc (ORCPT ); Tue, 15 Nov 2011 03:51:32 -0500 Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 15 Nov 2011 00:51:31 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.67,351,1309762800"; d="scan'208";a="75509503" Received: from minggr.sh.intel.com (HELO [10.239.36.47]) ([10.239.36.47]) by orsmga002.jf.intel.com with ESMTP; 15 Nov 2011 00:51:29 -0800 Subject: Re: [PATCH v2 4/4] ata: add ata port runtime PM callbacks From: Lin Ming To: Alan Stern Cc: lkml , "linux-ide@vger.kernel.org" , "linux-scsi@vger.kernel.org" , "linux-pm@vger.kernel.org" , Jeff Garzik , "Rafael J. Wysocki" , James Bottomley , Tejun Heo , "Huang, Ying" , "Zhang, Rui" In-Reply-To: References: Date: Tue, 15 Nov 2011 16:51:29 +0800 Message-ID: <1321347089.26084.2.camel@minggr> Mime-Version: 1.0 X-Mailer: Evolution 2.30.3 Sender: linux-ide-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ide@vger.kernel.org On Mon, 2011-11-14 at 23:31 +0800, Alan Stern wrote: > On Mon, 14 Nov 2011, Lin Ming wrote: > > > Current patches has a bug that system suspend fails if ata port was > > runtime suspended. > > > > disk suspend issues sync cache and stop device commands that obviously > > need ata port to be active. So we need to runtime resume ata port first. > > This is wrong. If the port is already suspended then so are all the > drives below the port. Hence there is no need to sync the cache or > stop the device. Ah, got it now! > > > Alan, Tejun > > > > How about below fix? > > > > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c > > index fa3a591..ebb87fbf 100644 > > --- a/drivers/scsi/sd.c > > +++ b/drivers/scsi/sd.c > > @@ -50,6 +50,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > > > @@ -2762,6 +2763,14 @@ static int sd_suspend(struct device *dev, pm_message_t mesg) > > if (!sdkp) > > return 0; /* this can happen */ > > > > + /* > > + * Resume parent device to handle sync cache and > > + * stop device commands > > + */ > > + ret = pm_runtime_get_sync(dev->parent); > > + if (ret) > > + goto exit; > > + > > if (sdkp->WCE) { > > sd_printk(KERN_NOTICE, sdkp, "Synchronizing SCSI cache\n"); > > ret = sd_sync_cache(sdkp); > > This is not the right approach. You should look instead at > scsi_dev_type_suspend() in scsi_pm.c. If the device is already runtime > suspended then the routine should return immediately. How about below? > > Alan Stern > --- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c index d329f8b..94b60bd 100644 --- a/drivers/scsi/scsi_pm.c +++ b/drivers/scsi/scsi_pm.c @@ -20,6 +20,9 @@ static int scsi_dev_type_suspend(struct device *dev, pm_message_t msg) struct device_driver *drv; int err; + if (pm_runtime_suspended(dev)) + return 0; + err = scsi_device_quiesce(to_scsi_device(dev)); if (err == 0) { drv = dev->driver;