| Message ID | 1410530416-30200-6-git-send-email-hanjun.guo@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 12, 2014 at 03:00:03PM +0100, Hanjun Guo wrote: > --- a/drivers/acpi/Makefile > +++ b/drivers/acpi/Makefile > @@ -23,7 +23,11 @@ acpi-y += nvs.o > > # Power management related files > acpi-y += wakeup.o > +ifeq ($(ARCH), arm64) > +acpi-y += sleep-arm.o > +else # X86, IA64 > acpi-y += sleep.o > +endif > acpi-y += device_pm.o > acpi-$(CONFIG_ACPI_SLEEP) += proc.o What about: -acpi-y += sleep.o +acpi-$(CONFIG_ACPI_SLEEP) += sleep.o and make CONFIG_ACPI_SLEEP depend on !ARM64? Does the kernel compile if you disable CONFIG_ACPI_SLEEP?
On Fri, Sep 12, 2014 at 03:51:02PM +0100, Catalin Marinas wrote: > On Fri, Sep 12, 2014 at 03:00:03PM +0100, Hanjun Guo wrote: > > --- a/drivers/acpi/Makefile > > +++ b/drivers/acpi/Makefile > > @@ -23,7 +23,11 @@ acpi-y += nvs.o > > > > # Power management related files > > acpi-y += wakeup.o > > +ifeq ($(ARCH), arm64) > > +acpi-y += sleep-arm.o > > +else # X86, IA64 > > acpi-y += sleep.o > > +endif > > acpi-y += device_pm.o > > acpi-$(CONFIG_ACPI_SLEEP) += proc.o > > What about: > > -acpi-y += sleep.o > +acpi-$(CONFIG_ACPI_SLEEP) += sleep.o > > and make CONFIG_ACPI_SLEEP depend on !ARM64? Does the kernel compile if > you disable CONFIG_ACPI_SLEEP? > sleep.c is still needed in the case CONFIG_ACPI_SLEEP=n basically the two functions I stubbed are always called by bus.c Graeme
On Fri, Sep 12, 2014 at 04:18:51PM +0100, Graeme Gregory wrote: > On Fri, Sep 12, 2014 at 03:51:02PM +0100, Catalin Marinas wrote: > > On Fri, Sep 12, 2014 at 03:00:03PM +0100, Hanjun Guo wrote: > > > --- a/drivers/acpi/Makefile > > > +++ b/drivers/acpi/Makefile > > > @@ -23,7 +23,11 @@ acpi-y += nvs.o > > > > > > # Power management related files > > > acpi-y += wakeup.o > > > +ifeq ($(ARCH), arm64) > > > +acpi-y += sleep-arm.o > > > +else # X86, IA64 > > > acpi-y += sleep.o > > > +endif > > > acpi-y += device_pm.o > > > acpi-$(CONFIG_ACPI_SLEEP) += proc.o > > > > What about: > > > > -acpi-y += sleep.o > > +acpi-$(CONFIG_ACPI_SLEEP) += sleep.o > > > > and make CONFIG_ACPI_SLEEP depend on !ARM64? Does the kernel compile if > > you disable CONFIG_ACPI_SLEEP? > > sleep.c is still needed in the case CONFIG_ACPI_SLEEP=n > > basically the two functions I stubbed are always called by bus.c So the two functions are: acpi_target_system_state() - already defined in acpi_bus.h when CONFIG_ACPI_SLEEP is off. acpi_sleep_init() - this one doesn't have a dummy implementation for !ACPI_SLEEP. Now, for x86, do we need acpi_sleep_init() when CONFIG_ACPI_SLEEP is off? If not, we could simply add a dummy acpi_sleep_init() function when !ACPI_SLEEP and don't enable the config option for arm64.
On Fri, Sep 12, 2014 at 04:49:03PM +0100, Catalin Marinas wrote: > On Fri, Sep 12, 2014 at 04:18:51PM +0100, Graeme Gregory wrote: > > On Fri, Sep 12, 2014 at 03:51:02PM +0100, Catalin Marinas wrote: > > > On Fri, Sep 12, 2014 at 03:00:03PM +0100, Hanjun Guo wrote: > > > > --- a/drivers/acpi/Makefile > > > > +++ b/drivers/acpi/Makefile > > > > @@ -23,7 +23,11 @@ acpi-y += nvs.o > > > > > > > > # Power management related files > > > > acpi-y += wakeup.o > > > > +ifeq ($(ARCH), arm64) > > > > +acpi-y += sleep-arm.o > > > > +else # X86, IA64 > > > > acpi-y += sleep.o > > > > +endif > > > > acpi-y += device_pm.o > > > > acpi-$(CONFIG_ACPI_SLEEP) += proc.o > > > > > > What about: > > > > > > -acpi-y += sleep.o > > > +acpi-$(CONFIG_ACPI_SLEEP) += sleep.o > > > > > > and make CONFIG_ACPI_SLEEP depend on !ARM64? Does the kernel compile if > > > you disable CONFIG_ACPI_SLEEP? > > > > sleep.c is still needed in the case CONFIG_ACPI_SLEEP=n > > > > basically the two functions I stubbed are always called by bus.c > > So the two functions are: > > acpi_target_system_state() - already defined in acpi_bus.h when > CONFIG_ACPI_SLEEP is off. > > acpi_sleep_init() - this one doesn't have a dummy implementation for > !ACPI_SLEEP. > > Now, for x86, do we need acpi_sleep_init() when CONFIG_ACPI_SLEEP is > off? If not, we could simply add a dummy acpi_sleep_init() function when > !ACPI_SLEEP and don't enable the config option for arm64. > It looks to me that it still does processing for the S5 state which is soft off. It sets up the operations to be called when entering this state and also sets up a notifier. Graeme
On 09/12/2014 10:00 AM, Hanjun Guo wrote: > From: Graeme Gregory <graeme.gregory@linaro.org> > > ACPI 5.1 does not currently support S states for ARM64 hardware but > ACPI code will call acpi_target_system_state() for device power > managment, so introduce sleep-arm.c to allow other drivers to function > until S states are defined. Aside: ACPI5.1 does define the Platform Communication Channel and CPC (Collaborative Processor Performance Control). Some details need to be fleshed out there for practical 64-bit ARMv8 server systems, but the underpinnings and mechanisms are in place in the 5.1 specification that can be built upon over time to achieve practical S-State equivalence. Jon.
On Fri, Sep 12, 2014 at 03:43:36PM -0400, Jon Masters wrote: > On 09/12/2014 10:00 AM, Hanjun Guo wrote: > > From: Graeme Gregory <graeme.gregory@linaro.org> > > > > ACPI 5.1 does not currently support S states for ARM64 hardware but > > ACPI code will call acpi_target_system_state() for device power > > managment, so introduce sleep-arm.c to allow other drivers to function > > until S states are defined. > > Aside: ACPI5.1 does define the Platform Communication Channel and CPC > (Collaborative Processor Performance Control). Some details need to be > fleshed out there for practical 64-bit ARMv8 server systems, but the > underpinnings and mechanisms are in place in the 5.1 specification that > can be built upon over time to achieve practical S-State equivalence. > Disagree, S-States are about going to sleep or hibernating, that is nothing to do with CPU Performance. Graeme
On 09/12/2014 04:03 PM, Graeme Gregory wrote: > On Fri, Sep 12, 2014 at 03:43:36PM -0400, Jon Masters wrote: >> On 09/12/2014 10:00 AM, Hanjun Guo wrote: >>> From: Graeme Gregory <graeme.gregory@linaro.org> >>> >>> ACPI 5.1 does not currently support S states for ARM64 hardware but >>> ACPI code will call acpi_target_system_state() for device power >>> managment, so introduce sleep-arm.c to allow other drivers to function >>> until S states are defined. >> >> Aside: ACPI5.1 does define the Platform Communication Channel and CPC >> (Collaborative Processor Performance Control). Some details need to be >> fleshed out there for practical 64-bit ARMv8 server systems, but the >> underpinnings and mechanisms are in place in the 5.1 specification that >> can be built upon over time to achieve practical S-State equivalence. ^ meant C/P sorry > Disagree, S-States are about going to sleep or hibernating, that is > nothing to do with CPU Performance. Correct. In replying to the above, I was thinking about power and performance management of components - CPU (C/P), and device (D) specific power/performance states - not the overall system (S) state, and I went off on a tangent. That was off-topic. For the actual topic at hand, that of global sleep, you are correct that only G0 (S0) and S5 are really defined today. Though I do think that the PCC might be used to actually implement overall system sleep state transitions later. Jon.
On Fri, Sep 12, 2014 at 04:49:03PM +0100, Catalin Marinas wrote: > Now, for x86, do we need acpi_sleep_init() when CONFIG_ACPI_SLEEP is > off? If not, we could simply add a dummy acpi_sleep_init() function when > !ACPI_SLEEP and don't enable the config option for arm64. The !ACPI_SLEEP case still uses the ACPI code for powering the system down. I'd recommend adding a new CONFIG_ACPI_POWER_OFF option, wrapping the remaining code in sleep.c and disabling that on ARM.
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 505d4d7..89837f8 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -23,7 +23,11 @@ acpi-y += nvs.o # Power management related files acpi-y += wakeup.o +ifeq ($(ARCH), arm64) +acpi-y += sleep-arm.o +else # X86, IA64 acpi-y += sleep.o +endif acpi-y += device_pm.o acpi-$(CONFIG_ACPI_SLEEP) += proc.o diff --git a/drivers/acpi/sleep-arm.c b/drivers/acpi/sleep-arm.c new file mode 100644 index 0000000..54578ef --- /dev/null +++ b/drivers/acpi/sleep-arm.c @@ -0,0 +1,28 @@ +/* + * ARM64 Specific Sleep Functionality + * + * Copyright (C) 2013-2014, Linaro Ltd. + * Author: Graeme Gregory <graeme.gregory@linaro.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/acpi.h> + +/* + * Currently the ACPI 5.1 standard does not define S states in a + * manner which is usable for ARM64. These two stubs are sufficient + * that system initialises and device PM works. + */ +u32 acpi_target_system_state(void) +{ + return ACPI_STATE_S0; +} +EXPORT_SYMBOL_GPL(acpi_target_system_state); + +int __init acpi_sleep_init(void) +{ + return -ENOSYS; +}