lkml.org 
[lkml]   [2018]   [Feb]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] PCMCIA / PM: Avoid noirq suspend aborts during suspend-to-idle
On Thu, Feb 22, 2018 at 5:45 PM, Dominik Brodowski
<linux@dominikbrodowski.net> wrote:
> 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().

I see.

> What about this variant (untested, not even compile-tested)?

It looks fine to me, I'll test it later today.

> Oh, and
> a) does this need -stable tags, and

Not right away I guess, but I will ask the -stable maintainers to pick
it up at one point if there are no problems reported with it.

> b) do you want to push it yourself, or should it go through the
> PCMCIA tree?

I'll take care of it.

Thanks,
Rafael


> ---
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 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 <rafael.j.wysocki@intel.com>
> [linux@dominikbrodowski.net: follow same codepaths for both suspend variants; call ->suspend()]
> Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
>
> 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

\
 
 \ /
  Last update: 2018-02-22 18:32    [W:0.067 / U:0.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site