Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755699AbcCUMnX (ORCPT ); Mon, 21 Mar 2016 08:43:23 -0400 Received: from mail-lf0-f46.google.com ([209.85.215.46]:35174 "EHLO mail-lf0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754585AbcCUMnV (ORCPT ); Mon, 21 Mar 2016 08:43:21 -0400 MIME-Version: 1.0 In-Reply-To: <1458484268-18311-1-git-send-email-yu.c.chen@intel.com> References: <1458484268-18311-1-git-send-email-yu.c.chen@intel.com> Date: Mon, 21 Mar 2016 13:43:18 +0100 X-Google-Sender-Auth: C3c0RsVnG5V9sfUaKQwFyaYLd9Y Message-ID: Subject: Re: [PATCH][v5] ACPI / PM: Introduce efi poweroff for HW-full platforms without _S5 From: "Rafael J. Wysocki" To: Chen Yu , Matt Fleming Cc: ACPI Devel Maling List , "Rafael J. Wysocki" , "linux-pm@vger.kernel.org" , linux-efi@vger.kernel.org, Linux Kernel Mailing List , x86@kernel.org, Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Len Brown Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3375 Lines: 92 On Sun, Mar 20, 2016 at 3:31 PM, Chen Yu wrote: > The problem is Linux registers pm_power_off = efi_power_off > only if we are in hardware reduced mode. Actually, what we also > want is to do this when ACPI S5 is simply not supported on > non-legacy platforms. Since there will be HW-full mode where > the DSDT fails to supply an _S5 object(without SLP_TYP), we > should let such kind of platform to leverage efi runtime service > to poweroff. > > This patch uses efi power off as first choice when S5 is > unavailable, even if there is a customized poweroff(driver provided, eg). > Meanwhile, the legacy platforms will not be affected because there is no > path for them to overwrite the pm_power_off to efi power off. > > Suggested-by: Len Brown > Signed-off-by: Chen Yu > --- > v5: > - Revert to version 2, enforce EFI poweroff for such kind > of platforms. > v4: > - Since in v3 efi_poweroff_required() is not guaranteed to run > after all of the other code that may register alternative > power off handling, add the pm_power_off_default that would > be used by pm_power_off if nothing else is available. > v3: > - Only assign pm_power_off to efi_power_off when there are no > other pm_power_off registered at that time, in case other > commponents would like to customize their own implementation. > --- > v2: > - Convert the acpi_no_s5 to a global bool variable in sleep.c and > add a declaration to include/linux/acpi.h. > --- > arch/x86/platform/efi/quirks.c | 2 +- > drivers/acpi/sleep.c | 7 +++++++ > include/linux/acpi.h | 1 + > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c > index ed30e79..7e5ec62 100644 > --- a/arch/x86/platform/efi/quirks.c > +++ b/arch/x86/platform/efi/quirks.c > @@ -340,5 +340,5 @@ bool efi_reboot_required(void) > > bool efi_poweroff_required(void) > { > - return !!acpi_gbl_reduced_hardware; > + return acpi_gbl_reduced_hardware || acpi_no_s5; > } > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c > index 9cb9752..a33859c 100644 > --- a/drivers/acpi/sleep.c > +++ b/drivers/acpi/sleep.c > @@ -25,6 +25,11 @@ > #include "internal.h" > #include "sleep.h" > > +/* > + * Some HW-full platforms do not have _S5, so they may need > + * to leverage efi power off for a shutdown. > + */ > +bool acpi_no_s5; > static u8 sleep_states[ACPI_S_STATE_COUNT]; > > static void acpi_sleep_tts_switch(u32 acpi_state) > @@ -846,6 +851,8 @@ int __init acpi_sleep_init(void) > sleep_states[ACPI_STATE_S5] = 1; > pm_power_off_prepare = acpi_power_off_prepare; > pm_power_off = acpi_power_off; > + } else { > + acpi_no_s5 = true; > } > > supported[0] = 0; > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index 06ed7e5..4d2e67f 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -278,6 +278,7 @@ void acpi_irq_stats_init(void); > extern u32 acpi_irq_handled; > extern u32 acpi_irq_not_handled; > extern unsigned int acpi_sci_irq; > +extern bool acpi_no_s5; > #define INVALID_ACPI_IRQ ((unsigned)-1) > static inline bool acpi_sci_irq_valid(void) > { > -- OK, this is fine by me. Matt, what do you think?