Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1382459 > unrolled thread

[PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI)

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) Sudeep Holla <sudeep.holla@arm.com> - 2016-04-19 14:40 +0200
    Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) kbuild test robot <lkp@intel.com> - 2016-04-19 16:10 +0200
      Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) Sudeep Holla <sudeep.holla@arm.com> - 2016-04-19 17:50 +0200
    Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) Vikas Sajjan <sajjan.linux@gmail.com> - 2016-04-20 12:10 +0200
      Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) Sudeep Holla <sudeep.holla@arm.com> - 2016-04-20 12:30 +0200
    Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) Jisheng Zhang <jszhang@marvell.com> - 2016-04-20 12:50 +0200
    Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) "Prakash, Prashanth" <pprakash@codeaurora.org> - 2016-04-26 18:00 +0200
      Re: [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI) Sudeep Holla <sudeep.holla@arm.com> - 2016-04-26 18:10 +0200

#1382459 — [PATCH v4 4/5] arm64: add support for ACPI Low Power Idle(LPI)

This patch adds appropriate callbacks to support ACPI Low Power Idle
(LPI) on ARM64.

It also selects ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI is enabled
on ARM64.

Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/Kconfig       |  1 +
 arch/arm64/kernel/acpi.c | 34 ++++++++++++++++++++++++++++++++++
 drivers/firmware/psci.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 4f436220384f..e7536540387d 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -10,6 +10,7 @@ config ARM64
 	select ARCH_HAS_SG_CHAIN
 	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
 	select ARCH_USE_CMPXCHG_LOCKREF
+	select ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI
 	select ARCH_SUPPORTS_ATOMIC_RMW
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index d1ce8e2f98b9..3c05ad5957be 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -18,6 +18,7 @@
 #include <linux/acpi.h>
 #include <linux/bootmem.h>
 #include <linux/cpumask.h>
+#include <linux/cpu_pm.h>
 #include <linux/init.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
@@ -25,9 +26,11 @@
 #include <linux/of_fdt.h>
 #include <linux/smp.h>
 
+#include <asm/cpuidle.h>
 #include <asm/cputype.h>
 #include <asm/cpu_ops.h>
 #include <asm/smp_plat.h>
+#include <acpi/processor.h>
 
 #ifdef CONFIG_ACPI_APEI
 # include <linux/efi.h>
@@ -211,6 +214,37 @@ void __init acpi_boot_table_init(void)
 	}
 }
 
+int acpi_processor_ffh_lpi_probe(unsigned int cpu)
+{
+	return arm_cpuidle_init(cpu);
+}
+
+struct acpi_processor_lpi *lpi;
+int acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx)
+{
+	int ret;
+
+	if (!idx) {
+		cpu_do_idle();
+		return idx;
+	}
+
+	/* TODO cpu_pm_{enter,exit} can be done in generic code ? */
+	ret = cpu_pm_enter();
+	if (!ret) {
+		/*
+		 * Pass idle state index to cpu_suspend which in turn will
+		 * call the CPU ops suspend protocol with idle index as a
+		 * parameter.
+		 */
+		ret = arm_cpuidle_suspend(idx);
+
+		cpu_pm_exit();
+	}
+
+	return ret ? -1 : idx;
+}
+
 #ifdef CONFIG_ACPI_APEI
 pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
 {
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index af6c5c839568..70cf2a500d4b 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -24,6 +24,7 @@
 #include <linux/reboot.h>
 #include <linux/slab.h>
 #include <linux/suspend.h>
+#include <linux/acpi.h>
 
 #include <uapi/linux/psci.h>
 
@@ -32,6 +33,7 @@
 #include <asm/system_misc.h>
 #include <asm/smp_plat.h>
 #include <asm/suspend.h>
+#include <acpi/processor.h>
 
 /*
  * While a 64-bit OS can make calls with SMC32 calling conventions, for some
@@ -316,8 +318,54 @@ static int psci_dt_cpu_init_idle(unsigned int cpu)
 	return ret;
 }
 
+static int __maybe_unused psci_acpi_cpu_init_idle(unsigned int cpu)
+{
+	int i, count;
+	u32 *psci_states;
+	struct acpi_processor *pr;
+	struct acpi_processor_lpi *lpi;
+
+	pr = per_cpu(processors, cpu);
+	if (unlikely(!pr || !pr->flags.has_lpi))
+		return -EINVAL;
+
+	/*
+	 * If the PSCI cpu_suspend function hook has not been initialized
+	 * idle states must not be enabled, so bail out
+	 */
+	if (!psci_ops.cpu_suspend)
+		return -EOPNOTSUPP;
+
+	count = pr->power.count - 1;
+	if (!count)
+		return -ENODEV;
+
+	psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL);
+	if (!psci_states)
+		return -ENOMEM;
+
+	for (i = 0; i < count; i++) {
+		u32 state;
+
+		lpi = &pr->power.lpi_states[i + 1];
+		state = lpi->address & 0xFFFFFFFF;
+		if (!psci_power_state_is_valid(state)) {
+			pr_warn("Invalid PSCI power state %#x\n", state);
+			kfree(psci_states);
+			return -EINVAL;
+		}
+		psci_states[i] = state;
+	}
+	/* Idle states parsed correctly, initialize per-cpu pointer */
+	per_cpu(psci_power_state, cpu) = psci_states;
+	return 0;
+}
+
 int psci_cpu_init_idle(unsigned int cpu)
 {
+	if (!acpi_disabled)
+		return psci_acpi_cpu_init_idle(cpu);
+
 	return psci_dt_cpu_init_idle(cpu);
 }
 
-- 
1.9.1

[toc] | [next] | [standalone]

#1382544

[Multipart message — attachments visible in raw view] — view raw

Hi,

[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.6-rc4 next-20160419]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Sudeep-Holla/ACPI-processor_idle-Add-ACPI-v6-0-LPI-support/20160419-203500
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: arm-sunxi_defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   In file included from drivers/firmware/psci.c:36:0:
>> include/acpi/processor.h:7:22: fatal error: asm/acpi.h: No such file or directory
    #include <asm/acpi.h>
                         ^
   compilation terminated.

vim +7 include/acpi/processor.h

^1da177e4c Linus Torvalds      2005-04-16   1  #ifndef __ACPI_PROCESSOR_H
^1da177e4c Linus Torvalds      2005-04-16   2  #define __ACPI_PROCESSOR_H
^1da177e4c Linus Torvalds      2005-04-16   3  
^1da177e4c Linus Torvalds      2005-04-16   4  #include <linux/kernel.h>
3b2d99429e Venkatesh Pallipadi 2005-12-14   5  #include <linux/cpu.h>
d9460fd227 Zhang Rui           2008-01-17   6  #include <linux/thermal.h>
02df8b9385 Venkatesh Pallipadi 2005-04-15  @7  #include <asm/acpi.h>
02df8b9385 Venkatesh Pallipadi 2005-04-15   8  
ac212b6980 Rafael J. Wysocki   2013-05-03   9  #define ACPI_PROCESSOR_CLASS		"processor"
ac212b6980 Rafael J. Wysocki   2013-05-03  10  #define ACPI_PROCESSOR_DEVICE_NAME	"Processor"

:::::: The code at line 7 was first introduced by commit
:::::: 02df8b9385c21fdba165bd380f60eca1d3b0578b [ACPI] enable C2 and C3 idle power states on SMP http://bugzilla.kernel.org/show_bug.cgi?id=4401

:::::: TO: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
:::::: CC: Len Brown <len.brown@intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1382622

On 19/04/16 14:59, kbuild test robot wrote:
> Hi,
>
> [auto build test ERROR on pm/linux-next]
> [also build test ERROR on v4.6-rc4 next-20160419]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>
> url:    https://github.com/0day-ci/linux/commits/Sudeep-Holla/ACPI-processor_idle-Add-ACPI-v6-0-LPI-support/20160419-203500
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
> config: arm-sunxi_defconfig (attached as .config)
> reproduce:
>          wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # save the attached .config to linux build tree
>          make.cross ARCH=arm
>
> All errors (new ones prefixed by >>):
>
>     In file included from drivers/firmware/psci.c:36:0:
>>> include/acpi/processor.h:7:22: fatal error: asm/acpi.h: No such file or directory
>      #include <asm/acpi.h>
>                           ^

Thanks again for the report, I had totally forgotten the need to check
ARM32 build. I now realize that PSCI driver is common now. Sorry for
that, fixed locally for now.

-- 
Regards,
Sudeep

[toc] | [prev] | [next] | [standalone]

#1383236

Hi Sudeep,

On Tue, Apr 19, 2016 at 6:00 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> This patch adds appropriate callbacks to support ACPI Low Power Idle
> (LPI) on ARM64.
>
> It also selects ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI is enabled
> on ARM64.
>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  arch/arm64/Kconfig       |  1 +
>  arch/arm64/kernel/acpi.c | 34 ++++++++++++++++++++++++++++++++++
>  drivers/firmware/psci.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 83 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 4f436220384f..e7536540387d 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -10,6 +10,7 @@ config ARM64
>         select ARCH_HAS_SG_CHAIN
>         select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>         select ARCH_USE_CMPXCHG_LOCKREF
> +       select ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI
>         select ARCH_SUPPORTS_ATOMIC_RMW
>         select ARCH_WANT_OPTIONAL_GPIOLIB
>         select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index d1ce8e2f98b9..3c05ad5957be 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -18,6 +18,7 @@
>  #include <linux/acpi.h>
>  #include <linux/bootmem.h>
>  #include <linux/cpumask.h>
> +#include <linux/cpu_pm.h>
>  #include <linux/init.h>
>  #include <linux/irq.h>
>  #include <linux/irqdomain.h>
> @@ -25,9 +26,11 @@
>  #include <linux/of_fdt.h>
>  #include <linux/smp.h>
>
> +#include <asm/cpuidle.h>
>  #include <asm/cputype.h>
>  #include <asm/cpu_ops.h>
>  #include <asm/smp_plat.h>
> +#include <acpi/processor.h>
>
>  #ifdef CONFIG_ACPI_APEI
>  # include <linux/efi.h>
> @@ -211,6 +214,37 @@ void __init acpi_boot_table_init(void)
>         }
>  }
>
> +int acpi_processor_ffh_lpi_probe(unsigned int cpu)
> +{
> +       return arm_cpuidle_init(cpu);
> +}
> +

This is generating warning as below:

WARNING: vmlinux.o(.text+0x11024): Section mismatch in reference from
the function acpi_processor_ffh_lpi_probe() to the function
.init.text:arm_cpuidle_init()
The function acpi_processor_ffh_lpi_probe() references
the function __init arm_cpuidle_init().
This is often because acpi_processor_ffh_lpi_probe lacks a __init
annotation or the annotation of arm_cpuidle_init is wrong.


> +struct acpi_processor_lpi *lpi;
> +int acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx)

Wondering how are you handling with Resource Dependencies for Idle.
I mean _RDI needs to be taken care, since the dependency between the
power resources and the LPI state is described in _RDI.

> +{
> +       int ret;
> +
> +       if (!idx) {
> +               cpu_do_idle();
> +               return idx;
> +       }
> +
> +       /* TODO cpu_pm_{enter,exit} can be done in generic code ? */
> +       ret = cpu_pm_enter();
> +       if (!ret) {
> +               /*
> +                * Pass idle state index to cpu_suspend which in turn will
> +                * call the CPU ops suspend protocol with idle index as a
> +                * parameter.
> +                */
> +               ret = arm_cpuidle_suspend(idx);
> +
> +               cpu_pm_exit();
> +       }
> +
> +       return ret ? -1 : idx;
> +}
> +
>  #ifdef CONFIG_ACPI_APEI
>  pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>  {
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index af6c5c839568..70cf2a500d4b 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -24,6 +24,7 @@
>  #include <linux/reboot.h>
>  #include <linux/slab.h>
>  #include <linux/suspend.h>
> +#include <linux/acpi.h>
>
>  #include <uapi/linux/psci.h>
>
> @@ -32,6 +33,7 @@
>  #include <asm/system_misc.h>
>  #include <asm/smp_plat.h>
>  #include <asm/suspend.h>
> +#include <acpi/processor.h>
>
>  /*
>   * While a 64-bit OS can make calls with SMC32 calling conventions, for some
> @@ -316,8 +318,54 @@ static int psci_dt_cpu_init_idle(unsigned int cpu)
>         return ret;
>  }
>
> +static int __maybe_unused psci_acpi_cpu_init_idle(unsigned int cpu)
> +{
> +       int i, count;
> +       u32 *psci_states;
> +       struct acpi_processor *pr;
> +       struct acpi_processor_lpi *lpi;
> +
> +       pr = per_cpu(processors, cpu);
> +       if (unlikely(!pr || !pr->flags.has_lpi))
> +               return -EINVAL;
> +
> +       /*
> +        * If the PSCI cpu_suspend function hook has not been initialized
> +        * idle states must not be enabled, so bail out
> +        */
> +       if (!psci_ops.cpu_suspend)
> +               return -EOPNOTSUPP;
> +
> +       count = pr->power.count - 1;
> +       if (!count)
> +               return -ENODEV;
> +
> +       psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL);
> +       if (!psci_states)
> +               return -ENOMEM;
> +
> +       for (i = 0; i < count; i++) {
> +               u32 state;
> +
> +               lpi = &pr->power.lpi_states[i + 1];
> +               state = lpi->address & 0xFFFFFFFF;
> +               if (!psci_power_state_is_valid(state)) {
> +                       pr_warn("Invalid PSCI power state %#x\n", state);
> +                       kfree(psci_states);
> +                       return -EINVAL;
> +               }
> +               psci_states[i] = state;
> +       }
> +       /* Idle states parsed correctly, initialize per-cpu pointer */
> +       per_cpu(psci_power_state, cpu) = psci_states;
> +       return 0;
> +}
> +
>  int psci_cpu_init_idle(unsigned int cpu)
>  {
> +       if (!acpi_disabled)
> +               return psci_acpi_cpu_init_idle(cpu);
> +
>         return psci_dt_cpu_init_idle(cpu);
>  }
>
> --
> 1.9.1
>

[toc] | [prev] | [next] | [standalone]

#1383245

On 20/04/16 10:59, Vikas Sajjan wrote:
> Hi Sudeep,
>
> On Tue, Apr 19, 2016 at 6:00 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> This patch adds appropriate callbacks to support ACPI Low Power Idle
>> (LPI) on ARM64.
>>
>> It also selects ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI is enabled
>> on ARM64.
>>

[...]

>> @@ -211,6 +214,37 @@ void __init acpi_boot_table_init(void)
>>          }
>>   }
>>
>> +int acpi_processor_ffh_lpi_probe(unsigned int cpu)
>> +{
>> +       return arm_cpuidle_init(cpu);
>> +}
>> +
>
> This is generating warning as below:
>
> WARNING: vmlinux.o(.text+0x11024): Section mismatch in reference from
> the function acpi_processor_ffh_lpi_probe() to the function
> .init.text:arm_cpuidle_init()
> The function acpi_processor_ffh_lpi_probe() references
> the function __init arm_cpuidle_init().
> This is often because acpi_processor_ffh_lpi_probe lacks a __init
> annotation or the annotation of arm_cpuidle_init is wrong.
>

I am aware of this and needs to be fixed. I posted ARM64/PSCI related
patches for completeness.

We can't have __init annotation for ..ffh_lpi_probe as it can be called
from hotplug paths in ACPI. Only solution I see is to remove __init tag
for arm_cpuidle_init. I raised similar concern on the other thread
yesterday[1]

Thanks for looking at these patches, much appreciated.

>
>> +struct acpi_processor_lpi *lpi;
>> +int acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx)
>
> Wondering how are you handling with Resource Dependencies for Idle.
> I mean _RDI needs to be taken care, since the dependency between the
> power resources and the LPI state is described in _RDI.
>

Correct, right now I haven't considered RDI yet as I don't have proper
platform to test. IMO it can be added later as RDI is optional and not
used on all platforms.

-- 
Regards,
Sudeep

[1] http://lkml.iu.edu/hypermail/linux/kernel/1604.2/02181.html

[toc] | [prev] | [next] | [standalone]

#1383251

Dear Sudeep,

On Tue, 19 Apr 2016 13:30:12 +0100 Sudeep Holla wrote:

> This patch adds appropriate callbacks to support ACPI Low Power Idle
> (LPI) on ARM64.
> 
> It also selects ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI is enabled
> on ARM64.
> 
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  arch/arm64/Kconfig       |  1 +
>  arch/arm64/kernel/acpi.c | 34 ++++++++++++++++++++++++++++++++++
>  drivers/firmware/psci.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 83 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 4f436220384f..e7536540387d 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -10,6 +10,7 @@ config ARM64
>  	select ARCH_HAS_SG_CHAIN
>  	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>  	select ARCH_USE_CMPXCHG_LOCKREF
> +	select ARCH_SUPPORTS_ACPI_PROCESSOR_LPI if ACPI
>  	select ARCH_SUPPORTS_ATOMIC_RMW
>  	select ARCH_WANT_OPTIONAL_GPIOLIB
>  	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index d1ce8e2f98b9..3c05ad5957be 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -18,6 +18,7 @@
>  #include <linux/acpi.h>
>  #include <linux/bootmem.h>
>  #include <linux/cpumask.h>
> +#include <linux/cpu_pm.h>
>  #include <linux/init.h>
>  #include <linux/irq.h>
>  #include <linux/irqdomain.h>
> @@ -25,9 +26,11 @@
>  #include <linux/of_fdt.h>
>  #include <linux/smp.h>
>  
> +#include <asm/cpuidle.h>
>  #include <asm/cputype.h>
>  #include <asm/cpu_ops.h>
>  #include <asm/smp_plat.h>
> +#include <acpi/processor.h>

It's better to keep headers alphabetic sorted

>  
>  #ifdef CONFIG_ACPI_APEI
>  # include <linux/efi.h>
> @@ -211,6 +214,37 @@ void __init acpi_boot_table_init(void)
>  	}
>  }
>  
> +int acpi_processor_ffh_lpi_probe(unsigned int cpu)
> +{
> +	return arm_cpuidle_init(cpu);
> +}
> +
> +struct acpi_processor_lpi *lpi;
> +int acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx)
> +{
> +	int ret;
> +
> +	if (!idx) {
> +		cpu_do_idle();
> +		return idx;
> +	}
> +
> +	/* TODO cpu_pm_{enter,exit} can be done in generic code ? */
> +	ret = cpu_pm_enter();
> +	if (!ret) {
> +		/*
> +		 * Pass idle state index to cpu_suspend which in turn will
> +		 * call the CPU ops suspend protocol with idle index as a
> +		 * parameter.
> +		 */
> +		ret = arm_cpuidle_suspend(idx);
> +
> +		cpu_pm_exit();
> +	}
> +
> +	return ret ? -1 : idx;
> +}
> +
>  #ifdef CONFIG_ACPI_APEI
>  pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>  {
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index af6c5c839568..70cf2a500d4b 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -24,6 +24,7 @@
>  #include <linux/reboot.h>
>  #include <linux/slab.h>
>  #include <linux/suspend.h>
> +#include <linux/acpi.h>

same here

>  
>  #include <uapi/linux/psci.h>
>  
> @@ -32,6 +33,7 @@
>  #include <asm/system_misc.h>
>  #include <asm/smp_plat.h>
>  #include <asm/suspend.h>
> +#include <acpi/processor.h>

ditto

>  
>  /*
>   * While a 64-bit OS can make calls with SMC32 calling conventions, for some
> @@ -316,8 +318,54 @@ static int psci_dt_cpu_init_idle(unsigned int cpu)
>  	return ret;
>  }
>  
> +static int __maybe_unused psci_acpi_cpu_init_idle(unsigned int cpu)
> +{
> +	int i, count;
> +	u32 *psci_states;
> +	struct acpi_processor *pr;
> +	struct acpi_processor_lpi *lpi;
> +
> +	pr = per_cpu(processors, cpu);
> +	if (unlikely(!pr || !pr->flags.has_lpi))
> +		return -EINVAL;
> +
> +	/*
> +	 * If the PSCI cpu_suspend function hook has not been initialized
> +	 * idle states must not be enabled, so bail out
> +	 */
> +	if (!psci_ops.cpu_suspend)
> +		return -EOPNOTSUPP;
> +
> +	count = pr->power.count - 1;
> +	if (!count)
> +		return -ENODEV;
> +
> +	psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL);
> +	if (!psci_states)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < count; i++) {
> +		u32 state;
> +
> +		lpi = &pr->power.lpi_states[i + 1];
> +		state = lpi->address & 0xFFFFFFFF;
> +		if (!psci_power_state_is_valid(state)) {
> +			pr_warn("Invalid PSCI power state %#x\n", state);
> +			kfree(psci_states);
> +			return -EINVAL;
> +		}
> +		psci_states[i] = state;
> +	}
> +	/* Idle states parsed correctly, initialize per-cpu pointer */
> +	per_cpu(psci_power_state, cpu) = psci_states;
> +	return 0;
> +}
> +
>  int psci_cpu_init_idle(unsigned int cpu)
>  {
> +	if (!acpi_disabled)
> +		return psci_acpi_cpu_init_idle(cpu);
> +
>  	return psci_dt_cpu_init_idle(cpu);
>  }
>  

[toc] | [prev] | [next] | [standalone]

#1387647

Hi Sudeep,

On 4/19/2016 6:30 AM, Sudeep Holla wrote:
> +struct acpi_processor_lpi *lpi;
> +int acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx)
> +{
> +	int ret;
> +
> +	if (!idx) {
> +		cpu_do_idle();
> +		return idx;
> +	}
> +
> +	/* TODO cpu_pm_{enter,exit} can be done in generic code ? */
> +	ret = cpu_pm_enter();
Can we avoid calling cpu_pm_enter and cpu_pm_exit for retention states as it is not necessary?
May be we can check LPI architecture specific context loss flags prior to calling these.
> +	if (!ret) {
> +		/*
> +		 * Pass idle state index to cpu_suspend which in turn will
> +		 * call the CPU ops suspend protocol with idle index as a
> +		 * parameter.
> +		 */
> +		ret = arm_cpuidle_suspend(idx);
> +
> +		cpu_pm_exit();
same here
> +	}
> +
> +	return ret ? -1 : idx;
> +}
> +

By the way, thanks for posting these patches!

-Prashanth

[toc] | [prev] | [next] | [standalone]

#1387656

Hi Prashanth,

On 26/04/16 16:51, Prakash, Prashanth wrote:
> Hi Sudeep,
>
> On 4/19/2016 6:30 AM, Sudeep Holla wrote:
>> +struct acpi_processor_lpi *lpi;
>> +int acpi_processor_ffh_lpi_enter(struct acpi_processor_lpi *lpi, int idx)
>> +{
>> +	int ret;
>> +
>> +	if (!idx) {
>> +		cpu_do_idle();
>> +		return idx;
>> +	}
>> +
>> +	/* TODO cpu_pm_{enter,exit} can be done in generic code ? */
>> +	ret = cpu_pm_enter();
> Can we avoid calling cpu_pm_enter and cpu_pm_exit for retention
> states as it is not necessary? May be we can check LPI architecture
> specific context loss flags prior to calling these.

Ah right, you had mentioned this before. Sorry for missing that.
Anyways, we need to get the driver reviewed before we can finalize arch
specific callbacks, so I will include this change when I post next version.

>> +	}
>> +
>> +	return ret ? -1 : idx;
>> +}
>> +
>
> By the way, thanks for posting these patches!
>

Thanks for taking look at these patches again :)

-- 
Regards,
Sudeep

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web