From patchwork Thu Feb 22 16:45:32 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dominik Brodowski X-Patchwork-Id: 10235953 X-Patchwork-Delegate: rjw@sisk.pl Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id CA28F602DC for ; Thu, 22 Feb 2018 16:47:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BCAFA28EC0 for ; Thu, 22 Feb 2018 16:47:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B142428EC2; Thu, 22 Feb 2018 16:47:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9F0DC28EC0 for ; Thu, 22 Feb 2018 16:47:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933219AbeBVQrX (ORCPT ); Thu, 22 Feb 2018 11:47:23 -0500 Received: from isilmar-4.linta.de ([136.243.71.142]:46042 "EHLO isilmar-4.linta.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933142AbeBVQrX (ORCPT ); Thu, 22 Feb 2018 11:47:23 -0500 Received: from light.dominikbrodowski.net (isilmar.linta [10.0.0.1]) by isilmar-4.linta.de (Postfix) with ESMTPS id 9028720090C; Thu, 22 Feb 2018 16:47:21 +0000 (UTC) Received: by light.dominikbrodowski.net (Postfix, from userid 1000) id 29E9420FAD; Thu, 22 Feb 2018 17:45:32 +0100 (CET) Date: Thu, 22 Feb 2018 17:45:32 +0100 From: Dominik Brodowski To: "Rafael J. Wysocki" Cc: Linux PM , LKML Subject: Re: [PATCH] PCMCIA / PM: Avoid noirq suspend aborts during suspend-to-idle Message-ID: <20180222164532.GA15359@light.dominikbrodowski.net> References: <4287858.bFmM5lK9H6@aspire.rjw.lan> <20180220213839.GA12082@light.dominikbrodowski.net> <3352336.6IBe4RuE7s@aspire.rjw.lan> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <3352336.6IBe4RuE7s@aspire.rjw.lan> User-Agent: Mutt/1.9.3 (2018-01-21) Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Feb 21, 2018 at 01:24:16PM +0100, Rafael J. Wysocki wrote: > Avoid that by using a new socket state flag, SOCKET_IN_RESUME, > to indicate that socket_early_resume() has already run for the > socket in which case socket_suspend() will do minimum handling > and return 0. That looks to be *too* minimal: For example, yenta_socket does some socket set-up in ->init(), which is called by socket_early_resume(). That needs to be undone by ->suspend(). What about this variant (untested, not even compile-tested)? Oh, and a) does this need -stable tags, and b) do you want to push it yourself, or should it go through the PCMCIA tree? Thanks, Dominik --- From: "Rafael J. Wysocki" Date: Wed, 21 Feb 2018 13:24:16 +0100 Subject: [PATCH] PCMCIA / PM: Avoid noirq suspend aborts during suspend-to-idle There is a problem with PCMCIA system resume callbacks with respect to suspend-to-idle in which the ->suspend_noirq() callback may be invoked after the ->resume_noirq() one without resuming the system entirely in some cases. This doesn't work for PCMCIA because of the lack of symmetry between its system suspend and system resume "noirq" callbacks. The system resume handling in PCMCIA is split between socket_early_resume() and socket_late_resume() which are called in different phases of system resume and both need to run for socket_suspend() (invoked by the system suspend "noirq" callback) to work. Specifically, socket_suspend() returns an error when called after socket_early_resume() without socket_late_resume(), so if the suspend-to-idle core detects a spurious wakeup event and attempts to put the system back to sleep, that is aborted by the error coming from socket_suspend(). Avoid that by using a new socket state flag, SOCKET_IN_RESUME, to indicate that socket_early_resume() has already run for the socket in which case socket_suspend() will do minimum handling and return 0. This change has been tested on my venerable Toshiba Portege R500 (which is where the problem has been discovered in the first place), but admittedly I have no PCMCIA cards to test along with the socket itself. Fixes: 33e4f80ee69b (ACPI / PM: Ignore spurious SCI wakeups from suspend-to-idle) Signed-off-by: Rafael J. Wysocki [linux@dominikbrodowski.net: follow same codepaths for both suspend variants; call ->suspend()] Signed-off-by: Dominik Brodowski diff --git a/drivers/pcmcia/cs.c b/drivers/pcmcia/cs.c index c3b615c94b4b..513b2d4d5b20 100644 --- a/drivers/pcmcia/cs.c +++ b/drivers/pcmcia/cs.c @@ -452,17 +452,20 @@ static int socket_insert(struct pcmcia_socket *skt) static int socket_suspend(struct pcmcia_socket *skt) { - if (skt->state & SOCKET_SUSPEND) + if ((skt->state & SOCKET_SUSPEND) && !(skt->state & SOCKET_IN_RESUME)) return -EBUSY; mutex_lock(&skt->ops_mutex); - skt->suspended_state = skt->state; + /* store state on first suspend, but not after spurious wakeups */ + if !(skt->state & SOCKET_IN_RESUME) + skt->suspended_state = skt->state; skt->socket = dead_socket; skt->ops->set_socket(skt, &skt->socket); if (skt->ops->suspend) skt->ops->suspend(skt); skt->state |= SOCKET_SUSPEND; + skt->state &= ~(SOCKET_IN_RESUME); mutex_unlock(&skt->ops_mutex); return 0; } @@ -475,6 +478,7 @@ static int socket_early_resume(struct pcmcia_socket *skt) skt->ops->set_socket(skt, &skt->socket); if (skt->state & SOCKET_PRESENT) skt->resume_status = socket_setup(skt, resume_delay); + skt->state |= SOCKET_IN_RESUME; mutex_unlock(&skt->ops_mutex); return 0; } @@ -484,7 +488,7 @@ static int socket_late_resume(struct pcmcia_socket *skt) int ret = 0; mutex_lock(&skt->ops_mutex); - skt->state &= ~SOCKET_SUSPEND; + skt->state &= ~(SOCKET_SUSPEND | SOCKET_IN_RESUME); mutex_unlock(&skt->ops_mutex); if (!(skt->state & SOCKET_PRESENT)) { diff --git a/drivers/pcmcia/cs_internal.h b/drivers/pcmcia/cs_internal.h index 6765beadea95..03ec43802909 100644 --- a/drivers/pcmcia/cs_internal.h +++ b/drivers/pcmcia/cs_internal.h @@ -70,6 +70,7 @@ struct pccard_resource_ops { /* Flags in socket state */ #define SOCKET_PRESENT 0x0008 #define SOCKET_INUSE 0x0010 +#define SOCKET_IN_RESUME 0x0040 #define SOCKET_SUSPEND 0x0080 #define SOCKET_WIN_REQ(i) (0x0100<<(i)) #define SOCKET_CARDBUS 0x8000