diff mbox

[v4,05/18] ARM64 / ACPI: Introduce sleep-arm.c

Message ID 1410530416-30200-6-git-send-email-hanjun.guo@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Hanjun Guo Sept. 12, 2014, 2 p.m. UTC
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.

Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 drivers/acpi/Makefile    |    4 ++++
 drivers/acpi/sleep-arm.c |   28 ++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)
 create mode 100644 drivers/acpi/sleep-arm.c

Comments

Catalin Marinas Sept. 12, 2014, 2:51 p.m. UTC | #1
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?
Graeme Gregory Sept. 12, 2014, 3:18 p.m. UTC | #2
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
Catalin Marinas Sept. 12, 2014, 3:49 p.m. UTC | #3
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.
Graeme Gregory Sept. 12, 2014, 4:32 p.m. UTC | #4
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
Jon Masters Sept. 12, 2014, 7:43 p.m. UTC | #5
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.
Graeme Gregory Sept. 12, 2014, 8:03 p.m. UTC | #6
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
Jon Masters Sept. 12, 2014, 9:10 p.m. UTC | #7
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.
Matthew Garrett Sept. 17, 2014, 1:31 a.m. UTC | #8
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 mbox

Patch

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;
+}