From patchwork Thu Oct 17 06:57:12 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aaron Lu X-Patchwork-Id: 3059091 Return-Path: X-Original-To: patchwork-linux-acpi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id E32319F3E2 for ; Thu, 17 Oct 2013 06:56:41 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4912F201BB for ; Thu, 17 Oct 2013 06:56:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6FEB7201EC for ; Thu, 17 Oct 2013 06:56:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751232Ab3JQG4g (ORCPT ); Thu, 17 Oct 2013 02:56:36 -0400 Received: from mga02.intel.com ([134.134.136.20]:21335 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750863Ab3JQG4f (ORCPT ); Thu, 17 Oct 2013 02:56:35 -0400 Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP; 16 Oct 2013 23:56:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.93,512,1378882800"; d="scan'208";a="394245405" Received: from aaronlu.sh.intel.com ([10.239.37.146]) by orsmga001.jf.intel.com with ESMTP; 16 Oct 2013 23:56:32 -0700 Message-ID: <525F8A48.9020007@intel.com> Date: Thu, 17 Oct 2013 14:57:12 +0800 From: Aaron Lu Organization: Intel, Shanghai, PRC MIME-Version: 1.0 To: "Rafael J. Wysocki" , ACPI Devel Maling List CC: Linux PM list , LKML , Lan Tianyu Subject: Re: [PATCH] ACPI / power: Drop automaitc resume of power resource dependent devices References: <4814451.BPLed2xxbP@vostro.rjw.lan> In-Reply-To: <4814451.BPLed2xxbP@vostro.rjw.lan> Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 10/16/2013 09:25 PM, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki > > The mechanism causing devices depending on a given power resource > (that is, devices that can be in D0 only if that power resource is > on) to be resumed automatically when the power resource is turned > on (and their "inferred" power state becomes D0 as a result) is > inherently racy and in fact unnecessary. > > It is racy, because if the power resources is turned on and then > immediately off, the device resume triggered by the first transition > to "on" may still happen, causing the power resource to be turned > on again. That again will trigger the "resume of dependent devices" > mechanism, but if the devices in question are not in use, they will > be suspended in the meantime causing the power resource to be turned > off. However, the "resume of dependent devices" will next resume > them again and so on. In some cases (USB port PM in particular) that > leads to an endless busy loop of flipping the resource on and off > continuously. > > It is needless, because whoever turns a power resource on will most > likely turn it off at some point and the devices that go into "D0" > as a result of turning it on will then go back into D3cold. > Moreover, turning all power resources a device needs to go into > D0 is not sufficient for a full transition into D0 in general. > Namely, _PS0 may need to be executed in addition to that in some > cases. This means that the whole rationale of the "resume of > dependent devices" mechanism was incorrect to begin with and it's > best to remove it entirely. With this patch, your previous patch could also be applied now: https://lkml.org/lkml/2013/6/14/173 [PATCH 4/4] ACPI / PM: Drop two functions that are not used any more And the following ATA patch at the same time: From: Aaron Lu Subject: [PATCH] ata: acpi: remove power dependent device handling Previously, we want the SCSI device corrsponding to ATA device gets runtime resumed when the power resource for that ATA device is turned on by some other device, so we added the SCSI device to the dependent device list of the ATA device's ACPI node. Since now we decided this behavior is troublesome and hard to avoid race, this behavior has been dropped. This patch cleans up the corresponding code in ATA ACPI layer. Signed-off-by: Aaron Lu --- drivers/ata/libata-acpi.c | 14 -------------- drivers/ata/libata-scsi.c | 3 --- drivers/ata/libata.h | 4 ---- 3 files changed, 21 deletions(-) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index 4ba8b04..ab714d2 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -1035,17 +1035,3 @@ void ata_acpi_on_disable(struct ata_device *dev) { ata_acpi_clear_gtf(dev); } - -void ata_scsi_acpi_bind(struct ata_device *dev) -{ - acpi_handle handle = ata_dev_acpi_handle(dev); - if (handle) - acpi_dev_pm_add_dependent(handle, &dev->sdev->sdev_gendev); -} - -void ata_scsi_acpi_unbind(struct ata_device *dev) -{ - acpi_handle handle = ata_dev_acpi_handle(dev); - if (handle) - acpi_dev_pm_remove_dependent(handle, &dev->sdev->sdev_gendev); -} diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 97a0cef..db6dfcf 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3679,7 +3679,6 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync) if (!IS_ERR(sdev)) { dev->sdev = sdev; scsi_device_put(sdev); - ata_scsi_acpi_bind(dev); } else { dev->sdev = NULL; } @@ -3767,8 +3766,6 @@ static void ata_scsi_remove_dev(struct ata_device *dev) struct scsi_device *sdev; unsigned long flags; - ata_scsi_acpi_unbind(dev); - /* Alas, we need to grab scan_mutex to ensure SCSI device * state doesn't change underneath us and thus * scsi_device_get() always succeeds. The mutex locking can diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h index eeeb778..45b5ab3 100644 --- a/drivers/ata/libata.h +++ b/drivers/ata/libata.h @@ -121,8 +121,6 @@ extern void ata_acpi_set_state(struct ata_port *ap, pm_message_t state); extern void ata_acpi_bind_port(struct ata_port *ap); extern void ata_acpi_bind_dev(struct ata_device *dev); extern acpi_handle ata_dev_acpi_handle(struct ata_device *dev); -extern void ata_scsi_acpi_bind(struct ata_device *dev); -extern void ata_scsi_acpi_unbind(struct ata_device *dev); #else static inline void ata_acpi_dissociate(struct ata_host *host) { } static inline int ata_acpi_on_suspend(struct ata_port *ap) { return 0; } @@ -133,8 +131,6 @@ static inline void ata_acpi_set_state(struct ata_port *ap, pm_message_t state) { } static inline void ata_acpi_bind_port(struct ata_port *ap) {} static inline void ata_acpi_bind_dev(struct ata_device *dev) {} -static inline void ata_scsi_acpi_bind(struct ata_device *dev) {} -static inline void ata_scsi_acpi_unbind(struct ata_device *dev) {} #endif /* libata-scsi.c */