* [RFC PATCH 0/3] arm: psci: implement cpuidle_ops
@ 2015-07-04 13:01 Jisheng Zhang
2015-07-04 13:01 ` [RFC PATCH 1/3] arm: psci: rename psci_smp.c to psci.c Jisheng Zhang
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Jisheng Zhang @ 2015-07-04 13:01 UTC (permalink / raw)
To: linux, will.deacon, mark.rutland, daniel.lezcano,
Catalin.Marinas, Lorenzo.Pieralisi
Cc: linux-arm-kernel, linux-kernel, Jisheng Zhang
We'd like to use cpuidle-arm.c for both arm and arm64 with psci as backend.
For arm64, it works. But for arm, we miss cpuidle_ops, these patches try to
address this issue.
Has been tested on Marvell Berlin SoCs. These patches are rebased on the Mark
Rutland's latest psci unification work.
[PATCH 1/3] renames psci_smp.c to psci.c to prepare for the next two changes.
[PATCH 2/3] enables PSCI on UP systems so that some calls like cpu_suspend can
be made functional on UP too
[PATCH 3/3] add the cpuidle_ops for psci. Most code is stolen from arm64, which
introduce duplicated psci code, that's why it's marked as "RFC", I dunno where
to put cpu_psci_cpu_init_idle() related functions.
Comments are welcome!
Jisheng Zhang (3):
arm: psci: rename psci_smp.c to psci.c
arm: psci: enable PSCI on UP systems
arm: psci: add cpuidle_ops support
arch/arm/kernel/Makefile | 5 +-
arch/arm/kernel/psci.c | 248 +++++++++++++++++++++++++++++++++++++++++++++
arch/arm/kernel/psci_smp.c | 130 ------------------------
3 files changed, 249 insertions(+), 134 deletions(-)
create mode 100644 arch/arm/kernel/psci.c
delete mode 100644 arch/arm/kernel/psci_smp.c
--
2.1.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFC PATCH 1/3] arm: psci: rename psci_smp.c to psci.c
2015-07-04 13:01 [RFC PATCH 0/3] arm: psci: implement cpuidle_ops Jisheng Zhang
@ 2015-07-04 13:01 ` Jisheng Zhang
2015-07-06 9:06 ` Daniel Lezcano
2015-07-04 13:01 ` [RFC PATCH 2/3] arm: psci: enable PSCI on UP systems Jisheng Zhang
2015-07-04 13:01 ` [RFC PATCH 3/3] arm: psci: add cpuidle_ops support Jisheng Zhang
2 siblings, 1 reply; 9+ messages in thread
From: Jisheng Zhang @ 2015-07-04 13:01 UTC (permalink / raw)
To: linux, will.deacon, mark.rutland, daniel.lezcano,
Catalin.Marinas, Lorenzo.Pieralisi
Cc: linux-arm-kernel, linux-kernel, Jisheng Zhang
This will prepare for next commit to enable PSCI for UP systems so that
calls like cpu_suspend can be made functional on UP too.
Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
---
arch/arm/kernel/Makefile | 2 +-
arch/arm/kernel/psci.c | 130 +++++++++++++++++++++++++++++++++++++++++++++
arch/arm/kernel/psci_smp.c | 130 ---------------------------------------------
3 files changed, 131 insertions(+), 131 deletions(-)
create mode 100644 arch/arm/kernel/psci.c
delete mode 100644 arch/arm/kernel/psci_smp.c
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 3b995f5..c57b2c0 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -90,7 +90,7 @@ obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
obj-$(CONFIG_ARM_VIRT_EXT) += hyp-stub.o
ifeq ($(CONFIG_ARM_PSCI),y)
obj-y += psci-call.o
-obj-$(CONFIG_SMP) += psci_smp.o
+obj-$(CONFIG_SMP) += psci.o
endif
extra-y := $(head-y) vmlinux.lds
diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
new file mode 100644
index 0000000..61c04b0
--- /dev/null
+++ b/arch/arm/kernel/psci.c
@@ -0,0 +1,130 @@
+/*
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Copyright (C) 2012 ARM Limited
+ *
+ * Author: Will Deacon <will.deacon@arm.com>
+ */
+
+#include <linux/init.h>
+#include <linux/smp.h>
+#include <linux/of.h>
+#include <linux/delay.h>
+#include <linux/psci.h>
+
+#include <uapi/linux/psci.h>
+
+#include <asm/psci.h>
+#include <asm/smp_plat.h>
+
+/*
+ * psci_smp assumes that the following is true about PSCI:
+ *
+ * cpu_suspend Suspend the execution on a CPU
+ * @state we don't currently describe affinity levels, so just pass 0.
+ * @entry_point the first instruction to be executed on return
+ * returns 0 success, < 0 on failure
+ *
+ * cpu_off Power down a CPU
+ * @state we don't currently describe affinity levels, so just pass 0.
+ * no return on successful call
+ *
+ * cpu_on Power up a CPU
+ * @cpuid cpuid of target CPU, as from MPIDR
+ * @entry_point the first instruction to be executed on return
+ * returns 0 success, < 0 on failure
+ *
+ * migrate Migrate the context to a different CPU
+ * @cpuid cpuid of target CPU, as from MPIDR
+ * returns 0 success, < 0 on failure
+ *
+ */
+
+extern void secondary_startup(void);
+
+static int psci_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+ if (psci_ops.cpu_on)
+ return psci_ops.cpu_on(cpu_logical_map(cpu),
+ virt_to_idmap(&secondary_startup));
+ return -ENODEV;
+}
+
+#ifdef CONFIG_HOTPLUG_CPU
+int psci_cpu_disable(unsigned int cpu)
+{
+ /* Fail early if we don't have CPU_OFF support */
+ if (!psci_ops.cpu_off)
+ return -EOPNOTSUPP;
+
+ /* Trusted OS will deny CPU_OFF */
+ if (psci_tos_resident_on(cpu))
+ return -EPERM;
+
+ return 0;
+}
+
+void __ref psci_cpu_die(unsigned int cpu)
+{
+ u32 state = PSCI_POWER_STATE_TYPE_POWER_DOWN <<
+ PSCI_0_2_POWER_STATE_TYPE_SHIFT;
+
+ if (psci_ops.cpu_off)
+ psci_ops.cpu_off(state);
+
+ /* We should never return */
+ panic("psci: cpu %d failed to shutdown\n", cpu);
+}
+
+int __ref psci_cpu_kill(unsigned int cpu)
+{
+ int err, i;
+
+ if (!psci_ops.affinity_info)
+ return 1;
+ /*
+ * cpu_kill could race with cpu_die and we can
+ * potentially end up declaring this cpu undead
+ * while it is dying. So, try again a few times.
+ */
+
+ for (i = 0; i < 10; i++) {
+ err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);
+ if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) {
+ pr_info("CPU%d killed.\n", cpu);
+ return 1;
+ }
+
+ msleep(10);
+ pr_info("Retrying again to check for CPU kill\n");
+ }
+
+ pr_warn("CPU%d may not have shut down cleanly (AFFINITY_INFO reports %d)\n",
+ cpu, err);
+ /* Make platform_cpu_kill() fail. */
+ return 0;
+}
+
+#endif
+
+bool __init psci_smp_available(void)
+{
+ /* is cpu_on available at least? */
+ return (psci_ops.cpu_on != NULL);
+}
+
+struct smp_operations __initdata psci_smp_ops = {
+ .smp_boot_secondary = psci_boot_secondary,
+#ifdef CONFIG_HOTPLUG_CPU
+ .cpu_disable = psci_cpu_disable,
+ .cpu_die = psci_cpu_die,
+ .cpu_kill = psci_cpu_kill,
+#endif
+};
diff --git a/arch/arm/kernel/psci_smp.c b/arch/arm/kernel/psci_smp.c
deleted file mode 100644
index 61c04b0..0000000
--- a/arch/arm/kernel/psci_smp.c
+++ /dev/null
@@ -1,130 +0,0 @@
-/*
- * 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.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * Copyright (C) 2012 ARM Limited
- *
- * Author: Will Deacon <will.deacon@arm.com>
- */
-
-#include <linux/init.h>
-#include <linux/smp.h>
-#include <linux/of.h>
-#include <linux/delay.h>
-#include <linux/psci.h>
-
-#include <uapi/linux/psci.h>
-
-#include <asm/psci.h>
-#include <asm/smp_plat.h>
-
-/*
- * psci_smp assumes that the following is true about PSCI:
- *
- * cpu_suspend Suspend the execution on a CPU
- * @state we don't currently describe affinity levels, so just pass 0.
- * @entry_point the first instruction to be executed on return
- * returns 0 success, < 0 on failure
- *
- * cpu_off Power down a CPU
- * @state we don't currently describe affinity levels, so just pass 0.
- * no return on successful call
- *
- * cpu_on Power up a CPU
- * @cpuid cpuid of target CPU, as from MPIDR
- * @entry_point the first instruction to be executed on return
- * returns 0 success, < 0 on failure
- *
- * migrate Migrate the context to a different CPU
- * @cpuid cpuid of target CPU, as from MPIDR
- * returns 0 success, < 0 on failure
- *
- */
-
-extern void secondary_startup(void);
-
-static int psci_boot_secondary(unsigned int cpu, struct task_struct *idle)
-{
- if (psci_ops.cpu_on)
- return psci_ops.cpu_on(cpu_logical_map(cpu),
- virt_to_idmap(&secondary_startup));
- return -ENODEV;
-}
-
-#ifdef CONFIG_HOTPLUG_CPU
-int psci_cpu_disable(unsigned int cpu)
-{
- /* Fail early if we don't have CPU_OFF support */
- if (!psci_ops.cpu_off)
- return -EOPNOTSUPP;
-
- /* Trusted OS will deny CPU_OFF */
- if (psci_tos_resident_on(cpu))
- return -EPERM;
-
- return 0;
-}
-
-void __ref psci_cpu_die(unsigned int cpu)
-{
- u32 state = PSCI_POWER_STATE_TYPE_POWER_DOWN <<
- PSCI_0_2_POWER_STATE_TYPE_SHIFT;
-
- if (psci_ops.cpu_off)
- psci_ops.cpu_off(state);
-
- /* We should never return */
- panic("psci: cpu %d failed to shutdown\n", cpu);
-}
-
-int __ref psci_cpu_kill(unsigned int cpu)
-{
- int err, i;
-
- if (!psci_ops.affinity_info)
- return 1;
- /*
- * cpu_kill could race with cpu_die and we can
- * potentially end up declaring this cpu undead
- * while it is dying. So, try again a few times.
- */
-
- for (i = 0; i < 10; i++) {
- err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);
- if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) {
- pr_info("CPU%d killed.\n", cpu);
- return 1;
- }
-
- msleep(10);
- pr_info("Retrying again to check for CPU kill\n");
- }
-
- pr_warn("CPU%d may not have shut down cleanly (AFFINITY_INFO reports %d)\n",
- cpu, err);
- /* Make platform_cpu_kill() fail. */
- return 0;
-}
-
-#endif
-
-bool __init psci_smp_available(void)
-{
- /* is cpu_on available at least? */
- return (psci_ops.cpu_on != NULL);
-}
-
-struct smp_operations __initdata psci_smp_ops = {
- .smp_boot_secondary = psci_boot_secondary,
-#ifdef CONFIG_HOTPLUG_CPU
- .cpu_disable = psci_cpu_disable,
- .cpu_die = psci_cpu_die,
- .cpu_kill = psci_cpu_kill,
-#endif
-};
--
2.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RFC PATCH 2/3] arm: psci: enable PSCI on UP systems
2015-07-04 13:01 [RFC PATCH 0/3] arm: psci: implement cpuidle_ops Jisheng Zhang
2015-07-04 13:01 ` [RFC PATCH 1/3] arm: psci: rename psci_smp.c to psci.c Jisheng Zhang
@ 2015-07-04 13:01 ` Jisheng Zhang
2015-07-07 14:16 ` Lorenzo Pieralisi
2015-07-04 13:01 ` [RFC PATCH 3/3] arm: psci: add cpuidle_ops support Jisheng Zhang
2 siblings, 1 reply; 9+ messages in thread
From: Jisheng Zhang @ 2015-07-04 13:01 UTC (permalink / raw)
To: linux, will.deacon, mark.rutland, daniel.lezcano,
Catalin.Marinas, Lorenzo.Pieralisi
Cc: linux-arm-kernel, linux-kernel, Jisheng Zhang
This is to make calls like eg cpu_suspend can be made functional on UP too.
Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
---
arch/arm/kernel/Makefile | 5 +----
arch/arm/kernel/psci.c | 2 ++
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index c57b2c0..2b1a60c 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -88,9 +88,6 @@ obj-$(CONFIG_DEBUG_LL) += debug.o
obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
obj-$(CONFIG_ARM_VIRT_EXT) += hyp-stub.o
-ifeq ($(CONFIG_ARM_PSCI),y)
-obj-y += psci-call.o
-obj-$(CONFIG_SMP) += psci.o
-endif
+obj-$(CONFIG_ARM_PSCI) += psci-call.o psci.o
extra-y := $(head-y) vmlinux.lds
diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
index 61c04b0..7f6ff02 100644
--- a/arch/arm/kernel/psci.c
+++ b/arch/arm/kernel/psci.c
@@ -47,6 +47,7 @@
*
*/
+#ifdef CONFIG_SMP
extern void secondary_startup(void);
static int psci_boot_secondary(unsigned int cpu, struct task_struct *idle)
@@ -128,3 +129,4 @@ struct smp_operations __initdata psci_smp_ops = {
.cpu_kill = psci_cpu_kill,
#endif
};
+#endif
--
2.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RFC PATCH 3/3] arm: psci: add cpuidle_ops support
2015-07-04 13:01 [RFC PATCH 0/3] arm: psci: implement cpuidle_ops Jisheng Zhang
2015-07-04 13:01 ` [RFC PATCH 1/3] arm: psci: rename psci_smp.c to psci.c Jisheng Zhang
2015-07-04 13:01 ` [RFC PATCH 2/3] arm: psci: enable PSCI on UP systems Jisheng Zhang
@ 2015-07-04 13:01 ` Jisheng Zhang
2015-07-07 14:23 ` Lorenzo Pieralisi
2 siblings, 1 reply; 9+ messages in thread
From: Jisheng Zhang @ 2015-07-04 13:01 UTC (permalink / raw)
To: linux, will.deacon, mark.rutland, daniel.lezcano,
Catalin.Marinas, Lorenzo.Pieralisi
Cc: linux-arm-kernel, linux-kernel, Jisheng Zhang
This patch implement cpuidle_ops using psci, the code is stolen from
arm64. Now we can use cpuidle-arm.c for both arm and arm64.
Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
---
arch/arm/kernel/psci.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 116 insertions(+)
diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
index 7f6ff02..7bf744d 100644
--- a/arch/arm/kernel/psci.c
+++ b/arch/arm/kernel/psci.c
@@ -13,16 +13,132 @@
* Author: Will Deacon <will.deacon@arm.com>
*/
+#include <linux/cpuidle.h>
#include <linux/init.h>
#include <linux/smp.h>
#include <linux/of.h>
#include <linux/delay.h>
#include <linux/psci.h>
+#include <linux/slab.h>
#include <uapi/linux/psci.h>
+#include <asm/cpuidle.h>
#include <asm/psci.h>
#include <asm/smp_plat.h>
+#include <asm/suspend.h>
+
+#ifdef CONFIG_CPU_IDLE
+static bool psci_power_state_loses_context(u32 state)
+{
+ return state & PSCI_0_2_POWER_STATE_TYPE_MASK;
+}
+
+static bool psci_power_state_is_valid(u32 state)
+{
+ const u32 valid_mask = PSCI_0_2_POWER_STATE_ID_MASK |
+ PSCI_0_2_POWER_STATE_TYPE_MASK |
+ PSCI_0_2_POWER_STATE_AFFL_MASK;
+
+ return !(state & ~valid_mask);
+}
+
+static DEFINE_PER_CPU_READ_MOSTLY(u32 *, psci_power_state);
+
+static int __init cpu_psci_cpu_init_idle(struct device_node *cpu_node,
+ int cpu)
+{
+ int i, ret, count = 0;
+ u32 *psci_states;
+ struct device_node *state_node;
+
+ /*
+ * 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 idle states */
+ while ((state_node = of_parse_phandle(cpu_node, "cpu-idle-states",
+ count))) {
+ count++;
+ of_node_put(state_node);
+ }
+
+ 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;
+
+ state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
+
+ ret = of_property_read_u32(state_node,
+ "arm,psci-suspend-param",
+ &state);
+ if (ret) {
+ pr_warn(" * %s missing arm,psci-suspend-param property\n",
+ state_node->full_name);
+ of_node_put(state_node);
+ goto free_mem;
+ }
+
+ of_node_put(state_node);
+ pr_warn("psci-power-state %#x index %d\n", state, i);
+ if (!psci_power_state_is_valid(state)) {
+ pr_warn("Invalid PSCI power state %#x\n", state);
+ ret = -EINVAL;
+ goto free_mem;
+ }
+ psci_states[i] = state;
+ }
+ /* Idle states parsed correctly, initialize per-cpu pointer */
+ per_cpu(psci_power_state, cpu) = psci_states;
+ return 0;
+
+free_mem:
+ kfree(psci_states);
+ return ret;
+}
+
+static int psci_suspend_finisher(unsigned long index)
+{
+ u32 *state = __this_cpu_read(psci_power_state);
+
+ return psci_ops.cpu_suspend(state[index - 1],
+ virt_to_phys(cpu_resume));
+}
+
+static int cpu_psci_cpu_suspend(int cpu, unsigned long index)
+{
+ int ret;
+ u32 *state = __this_cpu_read(psci_power_state);
+ /*
+ * idle state index 0 corresponds to wfi, should never be called
+ * from the cpu_suspend operations
+ */
+ if (WARN_ON_ONCE(!index))
+ return -EINVAL;
+
+ if (!psci_power_state_loses_context(state[index - 1]))
+ ret = psci_ops.cpu_suspend(state[index - 1], 0);
+ else
+ ret = cpu_suspend(index, psci_suspend_finisher);
+
+ return ret;
+}
+
+static struct cpuidle_ops psci_cpuidle_ops __initdata = {
+ .suspend = cpu_psci_cpu_suspend,
+ .init = cpu_psci_cpu_init_idle,
+};
+CPUIDLE_METHOD_OF_DECLARE(psci_idle, "psci", &psci_cpuidle_ops);
+#endif
/*
* psci_smp assumes that the following is true about PSCI:
--
2.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 1/3] arm: psci: rename psci_smp.c to psci.c
2015-07-04 13:01 ` [RFC PATCH 1/3] arm: psci: rename psci_smp.c to psci.c Jisheng Zhang
@ 2015-07-06 9:06 ` Daniel Lezcano
2015-07-06 9:14 ` Jisheng Zhang
0 siblings, 1 reply; 9+ messages in thread
From: Daniel Lezcano @ 2015-07-06 9:06 UTC (permalink / raw)
To: Jisheng Zhang, linux, will.deacon, mark.rutland, Catalin.Marinas,
Lorenzo.Pieralisi
Cc: linux-arm-kernel, linux-kernel
On 07/04/2015 03:01 PM, Jisheng Zhang wrote:
> This will prepare for next commit to enable PSCI for UP systems so that
> calls like cpu_suspend can be made functional on UP too.
>
> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> ---
> arch/arm/kernel/Makefile | 2 +-
> arch/arm/kernel/psci.c | 130 +++++++++++++++++++++++++++++++++++++++++++++
> arch/arm/kernel/psci_smp.c | 130 ---------------------------------------------
> 3 files changed, 131 insertions(+), 131 deletions(-)
> create mode 100644 arch/arm/kernel/psci.c
> delete mode 100644 arch/arm/kernel/psci_smp.c
When you move or rename a file, use the -M option with git when sending
the email. Instead of providing a big diff file, it will just annotate
the renaming.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 1/3] arm: psci: rename psci_smp.c to psci.c
2015-07-06 9:06 ` Daniel Lezcano
@ 2015-07-06 9:14 ` Jisheng Zhang
0 siblings, 0 replies; 9+ messages in thread
From: Jisheng Zhang @ 2015-07-06 9:14 UTC (permalink / raw)
To: Daniel Lezcano, linux, will.deacon, mark.rutland,
Catalin.Marinas, Lorenzo.Pieralisi
Cc: linux-arm-kernel, linux-kernel
On Mon, 6 Jul 2015 11:06:22 +0200
Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> On 07/04/2015 03:01 PM, Jisheng Zhang wrote:
> > This will prepare for next commit to enable PSCI for UP systems so that
> > calls like cpu_suspend can be made functional on UP too.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> > ---
> > arch/arm/kernel/Makefile | 2 +-
> > arch/arm/kernel/psci.c | 130 +++++++++++++++++++++++++++++++++++++++++++++
> > arch/arm/kernel/psci_smp.c | 130 ---------------------------------------------
> > 3 files changed, 131 insertions(+), 131 deletions(-)
> > create mode 100644 arch/arm/kernel/psci.c
> > delete mode 100644 arch/arm/kernel/psci_smp.c
>
> When you move or rename a file, use the -M option with git when sending
> the email. Instead of providing a big diff file, it will just annotate
> the renaming.
Thanks for the kindly remind and tips, I didn't know this before. Will take
care that in the future.
Thanks a lot,
Jisheng
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 2/3] arm: psci: enable PSCI on UP systems
2015-07-04 13:01 ` [RFC PATCH 2/3] arm: psci: enable PSCI on UP systems Jisheng Zhang
@ 2015-07-07 14:16 ` Lorenzo Pieralisi
0 siblings, 0 replies; 9+ messages in thread
From: Lorenzo Pieralisi @ 2015-07-07 14:16 UTC (permalink / raw)
To: Jisheng Zhang
Cc: linux, Will Deacon, Mark Rutland, daniel.lezcano,
Catalin Marinas, linux-arm-kernel, linux-kernel
On Sat, Jul 04, 2015 at 02:01:49PM +0100, Jisheng Zhang wrote:
> This is to make calls like eg cpu_suspend can be made functional on UP too.
>
> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> ---
> arch/arm/kernel/Makefile | 5 +----
> arch/arm/kernel/psci.c | 2 ++
> 2 files changed, 3 insertions(+), 4 deletions(-)
You will have to rewrite the commit log (mostly because PSCI CPU suspend
will move to drivers/firmware so it will become stale, I know what you
mean but it might be unclear to others so please improve it).
Patch (and intent) seems fine though:
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index c57b2c0..2b1a60c 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -88,9 +88,6 @@ obj-$(CONFIG_DEBUG_LL) += debug.o
> obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
>
> obj-$(CONFIG_ARM_VIRT_EXT) += hyp-stub.o
> -ifeq ($(CONFIG_ARM_PSCI),y)
> -obj-y += psci-call.o
> -obj-$(CONFIG_SMP) += psci.o
> -endif
> +obj-$(CONFIG_ARM_PSCI) += psci-call.o psci.o
>
> extra-y := $(head-y) vmlinux.lds
> diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
> index 61c04b0..7f6ff02 100644
> --- a/arch/arm/kernel/psci.c
> +++ b/arch/arm/kernel/psci.c
> @@ -47,6 +47,7 @@
> *
> */
>
> +#ifdef CONFIG_SMP
> extern void secondary_startup(void);
>
> static int psci_boot_secondary(unsigned int cpu, struct task_struct *idle)
> @@ -128,3 +129,4 @@ struct smp_operations __initdata psci_smp_ops = {
> .cpu_kill = psci_cpu_kill,
> #endif
> };
> +#endif
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 3/3] arm: psci: add cpuidle_ops support
2015-07-04 13:01 ` [RFC PATCH 3/3] arm: psci: add cpuidle_ops support Jisheng Zhang
@ 2015-07-07 14:23 ` Lorenzo Pieralisi
2015-07-08 6:46 ` Jisheng Zhang
0 siblings, 1 reply; 9+ messages in thread
From: Lorenzo Pieralisi @ 2015-07-07 14:23 UTC (permalink / raw)
To: Jisheng Zhang
Cc: linux, Will Deacon, Mark Rutland, daniel.lezcano,
Catalin Marinas, linux-arm-kernel, linux-kernel
On Sat, Jul 04, 2015 at 02:01:50PM +0100, Jisheng Zhang wrote:
> This patch implement cpuidle_ops using psci, the code is stolen from
> arm64. Now we can use cpuidle-arm.c for both arm and arm64.
You mean cpuidle-arm.c with PSCI back-end. You will have to rewrite
the commit log anyway since this patch will be significantly trimmed,
see below.
> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> ---
> arch/arm/kernel/psci.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 116 insertions(+)
>
> diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
> index 7f6ff02..7bf744d 100644
> --- a/arch/arm/kernel/psci.c
> +++ b/arch/arm/kernel/psci.c
> @@ -13,16 +13,132 @@
> * Author: Will Deacon <will.deacon@arm.com>
> */
>
> +#include <linux/cpuidle.h>
> #include <linux/init.h>
> #include <linux/smp.h>
> #include <linux/of.h>
> #include <linux/delay.h>
> #include <linux/psci.h>
> +#include <linux/slab.h>
>
> #include <uapi/linux/psci.h>
>
> +#include <asm/cpuidle.h>
> #include <asm/psci.h>
> #include <asm/smp_plat.h>
> +#include <asm/suspend.h>
> +
> +#ifdef CONFIG_CPU_IDLE
> +static bool psci_power_state_loses_context(u32 state)
> +{
> + return state & PSCI_0_2_POWER_STATE_TYPE_MASK;
> +}
> +
> +static bool psci_power_state_is_valid(u32 state)
> +{
> + const u32 valid_mask = PSCI_0_2_POWER_STATE_ID_MASK |
> + PSCI_0_2_POWER_STATE_TYPE_MASK |
> + PSCI_0_2_POWER_STATE_AFFL_MASK;
> +
> + return !(state & ~valid_mask);
> +}
Already moved these helpers to drivers/firmware:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/347355.html
> +
> +static DEFINE_PER_CPU_READ_MOSTLY(u32 *, psci_power_state);
> +
> +static int __init cpu_psci_cpu_init_idle(struct device_node *cpu_node,
> + int cpu)
> +{
> + int i, ret, count = 0;
> + u32 *psci_states;
> + struct device_node *state_node;
> +
> + /*
> + * 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 idle states */
> + while ((state_node = of_parse_phandle(cpu_node, "cpu-idle-states",
> + count))) {
> + count++;
> + of_node_put(state_node);
> + }
> +
> + 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;
> +
> + state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
> +
> + ret = of_property_read_u32(state_node,
> + "arm,psci-suspend-param",
> + &state);
> + if (ret) {
> + pr_warn(" * %s missing arm,psci-suspend-param property\n",
> + state_node->full_name);
> + of_node_put(state_node);
> + goto free_mem;
> + }
> +
> + of_node_put(state_node);
> + pr_warn("psci-power-state %#x index %d\n", state, i);
> + if (!psci_power_state_is_valid(state)) {
> + pr_warn("Invalid PSCI power state %#x\n", state);
> + ret = -EINVAL;
> + goto free_mem;
> + }
> + psci_states[i] = state;
> + }
> + /* Idle states parsed correctly, initialize per-cpu pointer */
> + per_cpu(psci_power_state, cpu) = psci_states;
> + return 0;
> +
> +free_mem:
> + kfree(psci_states);
> + return ret;
> +}
> +
> +static int psci_suspend_finisher(unsigned long index)
> +{
> + u32 *state = __this_cpu_read(psci_power_state);
> +
> + return psci_ops.cpu_suspend(state[index - 1],
> + virt_to_phys(cpu_resume));
> +}
> +
> +static int cpu_psci_cpu_suspend(int cpu, unsigned long index)
> +{
> + int ret;
> + u32 *state = __this_cpu_read(psci_power_state);
> + /*
> + * idle state index 0 corresponds to wfi, should never be called
> + * from the cpu_suspend operations
> + */
> + if (WARN_ON_ONCE(!index))
> + return -EINVAL;
> +
> + if (!psci_power_state_loses_context(state[index - 1]))
> + ret = psci_ops.cpu_suspend(state[index - 1], 0);
> + else
> + ret = cpu_suspend(index, psci_suspend_finisher);
> +
> + return ret;
> +}
Code above is arch agnostic and should be moved out of arch/arm (and
arm64).
Can you put together a patch to do it or you want me to do it ?
> +static struct cpuidle_ops psci_cpuidle_ops __initdata = {
> + .suspend = cpu_psci_cpu_suspend,
> + .init = cpu_psci_cpu_init_idle,
> +};
> +CPUIDLE_METHOD_OF_DECLARE(psci_idle, "psci", &psci_cpuidle_ops);
> +#endif
This is fine and should stay in arch/arm.
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 3/3] arm: psci: add cpuidle_ops support
2015-07-07 14:23 ` Lorenzo Pieralisi
@ 2015-07-08 6:46 ` Jisheng Zhang
0 siblings, 0 replies; 9+ messages in thread
From: Jisheng Zhang @ 2015-07-08 6:46 UTC (permalink / raw)
To: Lorenzo Pieralisi, daniel.lezcano
Cc: linux, Will Deacon, Mark Rutland, Catalin Marinas,
linux-arm-kernel, linux-kernel
Dear Lorenzo,
On Tue, 7 Jul 2015 15:23:45 +0100
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> On Sat, Jul 04, 2015 at 02:01:50PM +0100, Jisheng Zhang wrote:
> > This patch implement cpuidle_ops using psci, the code is stolen from
> > arm64. Now we can use cpuidle-arm.c for both arm and arm64.
>
> You mean cpuidle-arm.c with PSCI back-end. You will have to rewrite
> the commit log anyway since this patch will be significantly trimmed,
> see below.
Will do, thanks for the suggestion.
>
> > Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> > ---
> > arch/arm/kernel/psci.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 116 insertions(+)
> >
> > diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
> > index 7f6ff02..7bf744d 100644
> > --- a/arch/arm/kernel/psci.c
> > +++ b/arch/arm/kernel/psci.c
> > @@ -13,16 +13,132 @@
> > * Author: Will Deacon <will.deacon@arm.com>
> > */
> >
> > +#include <linux/cpuidle.h>
> > #include <linux/init.h>
> > #include <linux/smp.h>
> > #include <linux/of.h>
> > #include <linux/delay.h>
> > #include <linux/psci.h>
> > +#include <linux/slab.h>
> >
> > #include <uapi/linux/psci.h>
> >
> > +#include <asm/cpuidle.h>
> > #include <asm/psci.h>
> > #include <asm/smp_plat.h>
> > +#include <asm/suspend.h>
> > +
> > +#ifdef CONFIG_CPU_IDLE
> > +static bool psci_power_state_loses_context(u32 state)
> > +{
> > + return state & PSCI_0_2_POWER_STATE_TYPE_MASK;
> > +}
> > +
> > +static bool psci_power_state_is_valid(u32 state)
> > +{
> > + const u32 valid_mask = PSCI_0_2_POWER_STATE_ID_MASK |
> > + PSCI_0_2_POWER_STATE_TYPE_MASK |
> > + PSCI_0_2_POWER_STATE_AFFL_MASK;
> > +
> > + return !(state & ~valid_mask);
> > +}
>
> Already moved these helpers to drivers/firmware:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/347355.html
>
> > +
> > +static DEFINE_PER_CPU_READ_MOSTLY(u32 *, psci_power_state);
> > +
> > +static int __init cpu_psci_cpu_init_idle(struct device_node *cpu_node,
> > + int cpu)
> > +{
> > + int i, ret, count = 0;
> > + u32 *psci_states;
> > + struct device_node *state_node;
> > +
> > + /*
> > + * 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 idle states */
> > + while ((state_node = of_parse_phandle(cpu_node, "cpu-idle-states",
> > + count))) {
> > + count++;
> > + of_node_put(state_node);
> > + }
> > +
> > + 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;
> > +
> > + state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
> > +
> > + ret = of_property_read_u32(state_node,
> > + "arm,psci-suspend-param",
> > + &state);
> > + if (ret) {
> > + pr_warn(" * %s missing arm,psci-suspend-param property\n",
> > + state_node->full_name);
> > + of_node_put(state_node);
> > + goto free_mem;
> > + }
> > +
> > + of_node_put(state_node);
> > + pr_warn("psci-power-state %#x index %d\n", state, i);
> > + if (!psci_power_state_is_valid(state)) {
> > + pr_warn("Invalid PSCI power state %#x\n", state);
> > + ret = -EINVAL;
> > + goto free_mem;
> > + }
> > + psci_states[i] = state;
> > + }
> > + /* Idle states parsed correctly, initialize per-cpu pointer */
> > + per_cpu(psci_power_state, cpu) = psci_states;
> > + return 0;
> > +
> > +free_mem:
> > + kfree(psci_states);
> > + return ret;
> > +}
> > +
> > +static int psci_suspend_finisher(unsigned long index)
> > +{
> > + u32 *state = __this_cpu_read(psci_power_state);
> > +
> > + return psci_ops.cpu_suspend(state[index - 1],
> > + virt_to_phys(cpu_resume));
> > +}
> > +
> > +static int cpu_psci_cpu_suspend(int cpu, unsigned long index)
> > +{
> > + int ret;
> > + u32 *state = __this_cpu_read(psci_power_state);
> > + /*
> > + * idle state index 0 corresponds to wfi, should never be called
> > + * from the cpu_suspend operations
> > + */
> > + if (WARN_ON_ONCE(!index))
> > + return -EINVAL;
> > +
> > + if (!psci_power_state_loses_context(state[index - 1]))
> > + ret = psci_ops.cpu_suspend(state[index - 1], 0);
> > + else
> > + ret = cpu_suspend(index, psci_suspend_finisher);
> > +
> > + return ret;
> > +}
>
> Code above is arch agnostic and should be moved out of arch/arm (and
> arm64).
Got it. the remaining issue is cpuidle_ops.suspend prototype. Does it make
sense to change it from
int (*suspend)(int cpu, unsigned long arg);
to
int (*suspend)(unsigned long arg);
It seems that the to-be-suspended cpu is always the calling cpu itself.
Anyway I won't change the prototype but make a simple glue instead in the next
for patch for review.
>
> Can you put together a patch to do it or you want me to do it ?
>
> > +static struct cpuidle_ops psci_cpuidle_ops __initdata = {
> > + .suspend = cpu_psci_cpu_suspend,
> > + .init = cpu_psci_cpu_init_idle,
> > +};
> > +CPUIDLE_METHOD_OF_DECLARE(psci_idle, "psci", &psci_cpuidle_ops);
> > +#endif
>
> This is fine and should stay in arch/arm.
Thanks a lot for the review and tips, I'll cook newer patches for review.
Thanks,
Jisheng
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-07-08 6:47 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-04 13:01 [RFC PATCH 0/3] arm: psci: implement cpuidle_ops Jisheng Zhang
2015-07-04 13:01 ` [RFC PATCH 1/3] arm: psci: rename psci_smp.c to psci.c Jisheng Zhang
2015-07-06 9:06 ` Daniel Lezcano
2015-07-06 9:14 ` Jisheng Zhang
2015-07-04 13:01 ` [RFC PATCH 2/3] arm: psci: enable PSCI on UP systems Jisheng Zhang
2015-07-07 14:16 ` Lorenzo Pieralisi
2015-07-04 13:01 ` [RFC PATCH 3/3] arm: psci: add cpuidle_ops support Jisheng Zhang
2015-07-07 14:23 ` Lorenzo Pieralisi
2015-07-08 6:46 ` Jisheng Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).