2012-05-11 11:53:49

by Chenhui Zhao

[permalink] [raw]
Subject: [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync

Do hardware timebase sync. Firstly, stop all timebases, and transfer
the timebase value of the boot core to the other core. Finally,
start all timebases.

Only apply to dual-core chips, such as MPC8572, P2020, etc.

Signed-off-by: Zhao Chenhui <[email protected]>
Signed-off-by: Li Yang <[email protected]>
---
arch/powerpc/include/asm/fsl_guts.h | 2 +
arch/powerpc/platforms/85xx/smp.c | 93 +++++++++++++++++++++++++++++++++--
2 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/fsl_guts.h b/arch/powerpc/include/asm/fsl_guts.h
index aa4c488..dd5ba2c 100644
--- a/arch/powerpc/include/asm/fsl_guts.h
+++ b/arch/powerpc/include/asm/fsl_guts.h
@@ -48,6 +48,8 @@ struct ccsr_guts {
__be32 dmuxcr; /* 0x.0068 - DMA Mux Control Register */
u8 res06c[0x70 - 0x6c];
__be32 devdisr; /* 0x.0070 - Device Disable Control */
+#define CCSR_GUTS_DEVDISR_TB1 0x00001000
+#define CCSR_GUTS_DEVDISR_TB0 0x00004000
__be32 devdisr2; /* 0x.0074 - Device Disable Control 2 */
u8 res078[0x7c - 0x78];
__be32 pmjcr; /* 0x.007c - 4 Power Management Jog Control Register */
diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
index ff42490..6862dda 100644
--- a/arch/powerpc/platforms/85xx/smp.c
+++ b/arch/powerpc/platforms/85xx/smp.c
@@ -24,6 +24,7 @@
#include <asm/mpic.h>
#include <asm/cacheflush.h>
#include <asm/dbell.h>
+#include <asm/fsl_guts.h>

#include <sysdev/fsl_soc.h>
#include <sysdev/mpic.h>
@@ -115,13 +116,70 @@ smp_85xx_kick_cpu(int nr)

struct smp_ops_t smp_85xx_ops = {
.kick_cpu = smp_85xx_kick_cpu,
-#ifdef CONFIG_KEXEC
- .give_timebase = smp_generic_give_timebase,
- .take_timebase = smp_generic_take_timebase,
-#endif
};

#ifdef CONFIG_KEXEC
+static struct ccsr_guts __iomem *guts;
+static u64 timebase;
+static int tb_req;
+static int tb_valid;
+
+static void mpc85xx_timebase_freeze(int freeze)
+{
+ unsigned int mask;
+
+ if (!guts)
+ return;
+
+ mask = CCSR_GUTS_DEVDISR_TB0 | CCSR_GUTS_DEVDISR_TB1;
+ if (freeze)
+ setbits32(&guts->devdisr, mask);
+ else
+ clrbits32(&guts->devdisr, mask);
+
+ in_be32(&guts->devdisr);
+}
+
+static void mpc85xx_give_timebase(void)
+{
+ unsigned long flags;
+
+ local_irq_save(flags);
+
+ while (!tb_req)
+ barrier();
+ tb_req = 0;
+
+ mpc85xx_timebase_freeze(1);
+ timebase = get_tb();
+ mb();
+ tb_valid = 1;
+
+ while (tb_valid)
+ barrier();
+
+ mpc85xx_timebase_freeze(0);
+
+ local_irq_restore(flags);
+}
+
+static void mpc85xx_take_timebase(void)
+{
+ unsigned long flags;
+
+ local_irq_save(flags);
+
+ tb_req = 1;
+ while (!tb_valid)
+ barrier();
+
+ set_tb(timebase >> 32, timebase & 0xffffffff);
+ mb();
+ tb_valid = 0;
+
+ local_irq_restore(flags);
+}
+
atomic_t kexec_down_cpus = ATOMIC_INIT(0);

void mpc85xx_smp_kexec_cpu_down(int crash_shutdown, int secondary)
@@ -228,6 +286,20 @@ smp_85xx_setup_cpu(int cpu_nr)
doorbell_setup_this_cpu();
}

+#ifdef CONFIG_KEXEC
+static const struct of_device_id guts_ids[] = {
+ { .compatible = "fsl,mpc8572-guts", },
+ { .compatible = "fsl,mpc8560-guts", },
+ { .compatible = "fsl,mpc8536-guts", },
+ { .compatible = "fsl,p1020-guts", },
+ { .compatible = "fsl,p1021-guts", },
+ { .compatible = "fsl,p1022-guts", },
+ { .compatible = "fsl,p1023-guts", },
+ { .compatible = "fsl,p2020-guts", },
+ {},
+};
+#endif
+
void __init mpc85xx_smp_init(void)
{
struct device_node *np;
@@ -249,6 +321,19 @@ void __init mpc85xx_smp_init(void)
smp_85xx_ops.cause_ipi = doorbell_cause_ipi;
}

+#ifdef CONFIG_KEXEC
+ np = of_find_matching_node(NULL, guts_ids);
+ if (np) {
+ guts = of_iomap(np, 0);
+ smp_85xx_ops.give_timebase = mpc85xx_give_timebase;
+ smp_85xx_ops.take_timebase = mpc85xx_take_timebase;
+ of_node_put(np);
+ } else {
+ smp_85xx_ops.give_timebase = smp_generic_give_timebase;
+ smp_85xx_ops.take_timebase = smp_generic_take_timebase;
+ }
+#endif
+
smp_ops = &smp_85xx_ops;

#ifdef CONFIG_KEXEC
--
1.6.4.1


2012-05-11 11:54:46

by Chenhui Zhao

[permalink] [raw]
Subject: [PATCH v5 5/5] powerpc/85xx: add support to JOG feature using cpufreq interface

Some 85xx silicons like MPC8536 and P1022 have a JOG feature, which provides
a dynamic mechanism to lower or raise the CPU core clock at runtime.

This patch adds the support to change CPU frequency using the standard
cpufreq interface. The ratio CORE to CCB can be 1:1(except MPC8536), 3:2,
2:1, 5:2, 3:1, 7:2 and 4:1.

Two CPU cores on P1022 must not in the low power state during the frequency
transition. The driver uses a atomic counter to meet the requirement.

The jog mode frequency transition process on the MPC8536 is similar to
the deep sleep process. The driver need save the CPU state and restore
it after CPU warm reset.

Note:
* The I/O peripherals such as PCIe and eTSEC may lose packets during
the jog mode frequency transition.
* The driver doesn't support MPC8536 Rev 1.0 due to a JOG erratum.
Subsequent revisions of MPC8536 have corrected the erratum.

Signed-off-by: Dave Liu <[email protected]>
Signed-off-by: Li Yang <[email protected]>
Signed-off-by: Jerry Huang <[email protected]>
Signed-off-by: Zhao Chenhui <[email protected]>
CC: Scott Wood <[email protected]>
---
arch/powerpc/platforms/85xx/Makefile | 1 +
arch/powerpc/platforms/85xx/cpufreq-jog.c | 416 +++++++++++++++++++++++++++++
arch/powerpc/platforms/Kconfig | 11 +
arch/powerpc/sysdev/fsl_soc.h | 5 +
include/linux/cpu.h | 4 +
kernel/cpu.c | 60 ++--
6 files changed, 467 insertions(+), 30 deletions(-)
create mode 100644 arch/powerpc/platforms/85xx/cpufreq-jog.c

diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
index 12b526a..b49a0b6 100644
--- a/arch/powerpc/platforms/85xx/Makefile
+++ b/arch/powerpc/platforms/85xx/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_SMP) += smp.o
ifneq ($(CONFIG_PPC_E500MC),y)
obj-$(CONFIG_SUSPEND) += sleep.o
endif
+obj-$(CONFIG_MPC85xx_CPUFREQ) += cpufreq-jog.o

obj-y += common.o

diff --git a/arch/powerpc/platforms/85xx/cpufreq-jog.c b/arch/powerpc/platforms/85xx/cpufreq-jog.c
new file mode 100644
index 0000000..5d427ab
--- /dev/null
+++ b/arch/powerpc/platforms/85xx/cpufreq-jog.c
@@ -0,0 +1,416 @@
+/*
+ * Copyright (C) 2008-2012 Freescale Semiconductor, Inc.
+ * Author: Dave Liu <[email protected]>
+ * Modifier: Chenhui Zhao <[email protected]>
+ *
+ * The cpufreq driver is for Freescale 85xx processor,
+ * based on arch/powerpc/platforms/cell/cbe_cpufreq.c
+ * (C) Copyright IBM Deutschland Entwicklung GmbH 2005-2007
+ * Christian Krafft <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/cpufreq.h>
+#include <linux/of_platform.h>
+#include <linux/suspend.h>
+#include <linux/cpu.h>
+
+#include <asm/prom.h>
+#include <asm/time.h>
+#include <asm/reg.h>
+#include <asm/io.h>
+#include <asm/machdep.h>
+#include <asm/smp.h>
+
+#include <sysdev/fsl_soc.h>
+
+static DEFINE_MUTEX(mpc85xx_switch_mutex);
+static void __iomem *guts;
+
+static u32 sysfreq;
+static unsigned int max_pll[2];
+static atomic_t in_jog_process;
+static struct cpufreq_frequency_table *mpc85xx_freqs;
+static int (*set_pll)(unsigned int cpu, unsigned int pll);
+
+static struct cpufreq_frequency_table mpc8536_freqs_table[] = {
+ {3, 0},
+ {4, 0},
+ {5, 0},
+ {6, 0},
+ {7, 0},
+ {8, 0},
+ {0, CPUFREQ_TABLE_END},
+};
+
+static struct cpufreq_frequency_table p1022_freqs_table[] = {
+ {2, 0},
+ {3, 0},
+ {4, 0},
+ {5, 0},
+ {6, 0},
+ {7, 0},
+ {8, 0},
+ {0, CPUFREQ_TABLE_END},
+};
+
+#define FREQ_500MHz 500000000
+#define FREQ_800MHz 800000000
+
+#define CORE_RATIO_STRIDE 8
+#define CORE_RATIO_MASK 0x3f
+#define CORE_RATIO_SHIFT 16
+
+#define PORPLLSR 0x0 /* Power-On Reset PLL ratio status register */
+
+#define PMJCR 0x7c /* Power Management Jog Control Register */
+#define PMJCR_CORE0_SPD 0x00001000
+#define PMJCR_CORE_SPD 0x00002000
+
+#define POWMGTCSR 0x80 /* Power management control and status register */
+#define POWMGTCSR_JOG 0x00200000
+#define POWMGTCSR_INT_MASK 0x00000f00
+
+static void spin_while_jogging(void *dummy)
+{
+ unsigned long flags;
+
+ local_irq_save(flags);
+
+ atomic_inc(&in_jog_process);
+
+ while (atomic_read(&in_jog_process) != 0)
+ barrier();
+
+ local_irq_restore(flags);
+}
+
+static int get_pll(int hw_cpu)
+{
+ int shift;
+ u32 val = in_be32(guts + PORPLLSR);
+
+ shift = hw_cpu * CORE_RATIO_STRIDE + CORE_RATIO_SHIFT;
+
+ return (val >> shift) & CORE_RATIO_MASK;
+}
+
+static int mpc8536_set_pll(unsigned int cpu, unsigned int pll)
+{
+ u32 corefreq, val, mask;
+ unsigned int cur_pll = get_pll(0);
+ unsigned long flags;
+
+ if (pll == cur_pll)
+ return 0;
+
+ val = (pll & CORE_RATIO_MASK) << CORE_RATIO_SHIFT;
+
+ corefreq = sysfreq * pll / 2;
+ /*
+ * Set the COREx_SPD bit if the requested core frequency
+ * is larger than the threshold frequency.
+ */
+ if (corefreq > FREQ_800MHz)
+ val |= PMJCR_CORE_SPD;
+
+ mask = (CORE_RATIO_MASK << CORE_RATIO_SHIFT) | PMJCR_CORE_SPD;
+ clrsetbits_be32(guts + PMJCR, mask, val);
+
+ /* readback to sync write */
+ in_be32(guts + PMJCR);
+
+ local_irq_save(flags);
+ mpc85xx_enter_jog(get_immrbase(), POWMGTCSR_JOG);
+ local_irq_restore(flags);
+
+ /* verify */
+ cur_pll = get_pll(0);
+ if (cur_pll != pll) {
+ pr_err("%s: error. The current PLL of core 0 is %d instead of %d.\n",
+ __func__, cur_pll, pll);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int p1022_set_pll(unsigned int cpu, unsigned int pll)
+{
+ int index, hw_cpu = get_hard_smp_processor_id(cpu);
+ int shift;
+ u32 corefreq, val, mask = 0;
+ unsigned int cur_pll = get_pll(hw_cpu);
+ unsigned long flags;
+ int ret = 0;
+
+ if (pll == cur_pll)
+ return 0;
+
+ shift = hw_cpu * CORE_RATIO_STRIDE + CORE_RATIO_SHIFT;
+ val = (pll & CORE_RATIO_MASK) << shift;
+
+ corefreq = sysfreq * pll / 2;
+ /*
+ * Set the COREx_SPD bit if the requested core frequency
+ * is larger than the threshold frequency.
+ */
+ if (corefreq > FREQ_500MHz)
+ val |= PMJCR_CORE0_SPD << hw_cpu;
+
+ mask = (CORE_RATIO_MASK << shift) | (PMJCR_CORE0_SPD << hw_cpu);
+ clrsetbits_be32(guts + PMJCR, mask, val);
+
+ /* readback to sync write */
+ in_be32(guts + PMJCR);
+
+ cpu_hotplug_disable_before_freeze();
+ /*
+ * A Jog request can not be asserted when any core is in a low
+ * power state on P1022. Before executing a jog request, any
+ * core which is in a low power state must be waked by a
+ * interrupt, and keep waking up until the sequence is
+ * finished.
+ */
+ for_each_present_cpu(index) {
+ if (!cpu_online(index)) {
+ cpu_hotplug_enable_after_thaw();
+ pr_err("%s: error, core%d is down.\n", __func__, index);
+ return -1;
+ }
+ }
+
+ atomic_set(&in_jog_process, 0);
+ smp_call_function(spin_while_jogging, NULL, 0);
+
+ local_irq_save(flags);
+
+ /* Wait for the other core to wake. */
+ if (!spin_event_timeout(atomic_read(&in_jog_process) == 1, 1000, 100)) {
+ pr_err("%s: timeout, the other core is not at running state.\n",
+ __func__);
+ ret = -1;
+ goto err;
+ }
+
+ out_be32(guts + POWMGTCSR, POWMGTCSR_JOG | POWMGTCSR_INT_MASK);
+
+ if (!spin_event_timeout(
+ (in_be32(guts + POWMGTCSR) & POWMGTCSR_JOG) == 0, 1000, 100)) {
+ pr_err("%s: timeout, fail to switch the core frequency.\n",
+ __func__);
+ ret = -1;
+ goto err;
+ }
+
+ clrbits32(guts + POWMGTCSR, POWMGTCSR_INT_MASK);
+ in_be32(guts + POWMGTCSR);
+
+ atomic_set(&in_jog_process, 0);
+err:
+ local_irq_restore(flags);
+ cpu_hotplug_enable_after_thaw();
+
+ /* verify */
+ cur_pll = get_pll(hw_cpu);
+ if (cur_pll != pll) {
+ pr_err("%s: error, the current PLL of core %d is %d instead of %d.\n",
+ __func__, hw_cpu, cur_pll, pll);
+ return -1;
+ }
+
+ return ret;
+}
+
+/*
+ * cpufreq functions
+ */
+static int mpc85xx_cpufreq_cpu_init(struct cpufreq_policy *policy)
+{
+ unsigned int i, cur_pll;
+ int hw_cpu = get_hard_smp_processor_id(policy->cpu);
+
+ if (!cpu_present(policy->cpu))
+ return -ENODEV;
+
+ /* the latency of a transition, the unit is ns */
+ policy->cpuinfo.transition_latency = 2000;
+
+ cur_pll = get_pll(hw_cpu);
+
+ /* initialize frequency table */
+ pr_debug("core%d frequency table:\n", hw_cpu);
+ for (i = 0; mpc85xx_freqs[i].frequency != CPUFREQ_TABLE_END; i++) {
+ if (mpc85xx_freqs[i].index <= max_pll[hw_cpu]) {
+ /* The frequency unit is kHz. */
+ mpc85xx_freqs[i].frequency =
+ (sysfreq * mpc85xx_freqs[i].index / 2) / 1000;
+ } else {
+ mpc85xx_freqs[i].frequency = CPUFREQ_ENTRY_INVALID;
+ }
+
+ pr_debug("%d: %dkHz\n", i, mpc85xx_freqs[i].frequency);
+
+ if (mpc85xx_freqs[i].index == cur_pll)
+ policy->cur = mpc85xx_freqs[i].frequency;
+ }
+ pr_debug("current pll is at %d, and core freq is%d\n",
+ cur_pll, policy->cur);
+
+ cpufreq_frequency_table_get_attr(mpc85xx_freqs, policy->cpu);
+
+ /*
+ * This ensures that policy->cpuinfo_min
+ * and policy->cpuinfo_max are set correctly.
+ */
+ return cpufreq_frequency_table_cpuinfo(policy, mpc85xx_freqs);
+}
+
+static int mpc85xx_cpufreq_cpu_exit(struct cpufreq_policy *policy)
+{
+ cpufreq_frequency_table_put_attr(policy->cpu);
+
+ return 0;
+}
+
+static int mpc85xx_cpufreq_verify(struct cpufreq_policy *policy)
+{
+ return cpufreq_frequency_table_verify(policy, mpc85xx_freqs);
+}
+
+static int mpc85xx_cpufreq_target(struct cpufreq_policy *policy,
+ unsigned int target_freq,
+ unsigned int relation)
+{
+ struct cpufreq_freqs freqs;
+ unsigned int new;
+ int ret = 0;
+
+ if (!set_pll)
+ return -ENODEV;
+
+ cpufreq_frequency_table_target(policy,
+ mpc85xx_freqs,
+ target_freq,
+ relation,
+ &new);
+
+ freqs.old = policy->cur;
+ freqs.new = mpc85xx_freqs[new].frequency;
+ freqs.cpu = policy->cpu;
+
+ mutex_lock(&mpc85xx_switch_mutex);
+ cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+
+ ret = set_pll(policy->cpu, mpc85xx_freqs[new].index);
+ if (!ret) {
+ pr_info("cpufreq: Setting core%d frequency to %d kHz and PLL ratio to %d:2\n",
+ policy->cpu, mpc85xx_freqs[new].frequency,
+ mpc85xx_freqs[new].index);
+
+ ppc_proc_freq = freqs.new * 1000ul;
+ }
+ cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+ mutex_unlock(&mpc85xx_switch_mutex);
+
+ return ret;
+}
+
+static struct cpufreq_driver mpc85xx_cpufreq_driver = {
+ .verify = mpc85xx_cpufreq_verify,
+ .target = mpc85xx_cpufreq_target,
+ .init = mpc85xx_cpufreq_cpu_init,
+ .exit = mpc85xx_cpufreq_cpu_exit,
+ .name = "mpc85xx-JOG",
+ .owner = THIS_MODULE,
+ .flags = CPUFREQ_CONST_LOOPS,
+};
+
+static int mpc85xx_job_probe(struct platform_device *ofdev)
+{
+ struct device_node *np = ofdev->dev.of_node;
+ unsigned int svr;
+
+ if (of_device_is_compatible(np, "fsl,mpc8536-guts")) {
+ svr = mfspr(SPRN_SVR);
+ if ((svr & 0x7fff) == 0x10) {
+ pr_err("MPC8536 Rev 1.0 do not support JOG.\n");
+ return -ENODEV;
+ }
+ mpc85xx_freqs = mpc8536_freqs_table;
+ set_pll = mpc8536_set_pll;
+ } else if (of_device_is_compatible(np, "fsl,p1022-guts")) {
+ mpc85xx_freqs = p1022_freqs_table;
+ set_pll = p1022_set_pll;
+ } else {
+ return -ENODEV;
+ }
+
+ sysfreq = fsl_get_sys_freq();
+
+ guts = of_iomap(np, 0);
+ if (!guts)
+ return -ENODEV;
+
+ max_pll[0] = get_pll(0);
+ if (mpc85xx_freqs == p1022_freqs_table)
+ max_pll[1] = get_pll(1);
+
+ pr_info("Freescale MPC85xx CPU frequency switching(JOG) driver\n");
+
+ return cpufreq_register_driver(&mpc85xx_cpufreq_driver);
+}
+
+static int mpc85xx_jog_remove(struct platform_device *ofdev)
+{
+ iounmap(guts);
+ cpufreq_unregister_driver(&mpc85xx_cpufreq_driver);
+
+ return 0;
+}
+
+static struct of_device_id mpc85xx_jog_ids[] = {
+ { .compatible = "fsl,mpc8536-guts", },
+ { .compatible = "fsl,p1022-guts", },
+ {}
+};
+
+static struct platform_driver mpc85xx_jog_driver = {
+ .driver = {
+ .name = "mpc85xx_cpufreq_jog",
+ .owner = THIS_MODULE,
+ .of_match_table = mpc85xx_jog_ids,
+ },
+ .probe = mpc85xx_job_probe,
+ .remove = mpc85xx_jog_remove,
+};
+
+static int __init mpc85xx_jog_init(void)
+{
+ return platform_driver_register(&mpc85xx_jog_driver);
+}
+
+static void __exit mpc85xx_jog_exit(void)
+{
+ platform_driver_unregister(&mpc85xx_jog_driver);
+}
+
+module_init(mpc85xx_jog_init);
+module_exit(mpc85xx_jog_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Dave Liu <[email protected]>");
diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index a35ca44..445bedd 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -204,6 +204,17 @@ config CPU_FREQ_PMAC64
This adds support for frequency switching on Apple iMac G5,
and some of the more recent desktop G5 machines as well.

+config MPC85xx_CPUFREQ
+ bool "Support for Freescale MPC85xx CPU freq"
+ depends on PPC_85xx && PPC32 && !PPC_E500MC
+ default y
+ select CPU_FREQ_TABLE
+ help
+ This adds support for dynamic frequency switching on
+ Freescale MPC85xx by cpufreq interface. MPC8536 and P1022
+ have a JOG feature, which provides a dynamic mechanism
+ to lower or raise the CPU core clock at runtime.
+
config PPC_PASEMI_CPUFREQ
bool "Support for PA Semi PWRficient"
depends on PPC_PASEMI
diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
index 8976534..401cac0 100644
--- a/arch/powerpc/sysdev/fsl_soc.h
+++ b/arch/powerpc/sysdev/fsl_soc.h
@@ -62,5 +62,10 @@ void fsl_hv_halt(void);
* code can be compatible with both 32-bit & 36-bit.
*/
extern void mpc85xx_enter_deep_sleep(u64 ccsrbar, u32 powmgtreq);
+
+static inline void mpc85xx_enter_jog(u64 ccsrbar, u32 powmgtreq)
+{
+ mpc85xx_enter_deep_sleep(ccsrbar, powmgtreq);
+}
#endif
#endif
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index ee28844..eceb399 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -147,6 +147,8 @@ void notify_cpu_starting(unsigned int cpu);
extern void cpu_maps_update_begin(void);
extern void cpu_maps_update_done(void);

+extern void cpu_hotplug_disable_before_freeze(void);
+extern void cpu_hotplug_enable_after_thaw(void);
#else /* CONFIG_SMP */

#define cpu_notifier(fn, pri) do { (void)(fn); } while (0)
@@ -168,6 +170,8 @@ static inline void cpu_maps_update_done(void)
{
}

+static inline void cpu_hotplug_disable_before_freeze(void) {}
+static inline void cpu_hotplug_enable_after_thaw(void) {}
#endif /* CONFIG_SMP */
extern struct bus_type cpu_subsys;

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 2060c6e..c4cd342 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -381,6 +381,36 @@ out:
}
EXPORT_SYMBOL_GPL(cpu_up);

+/*
+ * Prevent regular CPU hotplug from racing with the freezer, by disabling CPU
+ * hotplug when tasks are about to be frozen. Also, don't allow the freezer
+ * to continue until any currently running CPU hotplug operation gets
+ * completed.
+ * To modify the 'cpu_hotplug_disabled' flag, we need to acquire the
+ * 'cpu_add_remove_lock'. And this same lock is also taken by the regular
+ * CPU hotplug path and released only after it is complete. Thus, we
+ * (and hence the freezer) will block here until any currently running CPU
+ * hotplug operation gets completed.
+ */
+void cpu_hotplug_disable_before_freeze(void)
+{
+ cpu_maps_update_begin();
+ cpu_hotplug_disabled = 1;
+ cpu_maps_update_done();
+}
+
+
+/*
+ * When tasks have been thawed, re-enable regular CPU hotplug (which had been
+ * disabled while beginning to freeze tasks).
+ */
+void cpu_hotplug_enable_after_thaw(void)
+{
+ cpu_maps_update_begin();
+ cpu_hotplug_disabled = 0;
+ cpu_maps_update_done();
+}
+
#ifdef CONFIG_PM_SLEEP_SMP
static cpumask_var_t frozen_cpus;

@@ -479,36 +509,6 @@ static int __init alloc_frozen_cpus(void)
core_initcall(alloc_frozen_cpus);

/*
- * Prevent regular CPU hotplug from racing with the freezer, by disabling CPU
- * hotplug when tasks are about to be frozen. Also, don't allow the freezer
- * to continue until any currently running CPU hotplug operation gets
- * completed.
- * To modify the 'cpu_hotplug_disabled' flag, we need to acquire the
- * 'cpu_add_remove_lock'. And this same lock is also taken by the regular
- * CPU hotplug path and released only after it is complete. Thus, we
- * (and hence the freezer) will block here until any currently running CPU
- * hotplug operation gets completed.
- */
-void cpu_hotplug_disable_before_freeze(void)
-{
- cpu_maps_update_begin();
- cpu_hotplug_disabled = 1;
- cpu_maps_update_done();
-}
-
-
-/*
- * When tasks have been thawed, re-enable regular CPU hotplug (which had been
- * disabled while beginning to freeze tasks).
- */
-void cpu_hotplug_enable_after_thaw(void)
-{
- cpu_maps_update_begin();
- cpu_hotplug_disabled = 0;
- cpu_maps_update_done();
-}
-
-/*
* When callbacks for CPU hotplug notifications are being executed, we must
* ensure that the state of the system with respect to the tasks being frozen
* or not, as reported by the notification, remains unchanged *throughout the
--
1.6.4.1

2012-05-11 11:54:19

by Chenhui Zhao

[permalink] [raw]
Subject: [PATCH v5 2/5] powerpc/85xx: add HOTPLUG_CPU support

From: Li Yang <[email protected]>

Add support to disable and re-enable individual cores at runtime
on MPC85xx/QorIQ SMP machines. Currently support e500v1/e500v2 core.

MPC85xx machines use ePAPR spin-table in boot page for CPU kick-off.
This patch uses the boot page from bootloader to boot core at runtime.
It supports 32-bit and 36-bit physical address.

Add generic_set_cpu_up() to set cpu_state as CPU_UP_PREPARE in kick_cpu().

Signed-off-by: Li Yang <[email protected]>
Signed-off-by: Jin Qing <[email protected]>
Signed-off-by: Zhao Chenhui <[email protected]>
---
Changes for v5:
* Used hardware timebase sync.

arch/powerpc/Kconfig | 6 +-
arch/powerpc/include/asm/cacheflush.h | 4 +
arch/powerpc/include/asm/smp.h | 2 +
arch/powerpc/kernel/head_fsl_booke.S | 28 +++++++
arch/powerpc/kernel/smp.c | 10 +++
arch/powerpc/platforms/85xx/smp.c | 145 ++++++++++++++++++++++++---------
6 files changed, 153 insertions(+), 42 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index feab3ba..d65ae35 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -220,7 +220,8 @@ config ARCH_HIBERNATION_POSSIBLE
config ARCH_SUSPEND_POSSIBLE
def_bool y
depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || \
- (PPC_85xx && !SMP) || PPC_86xx || PPC_PSERIES || 44x || 40x
+ (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES \
+ || 44x || 40x

config PPC_DCR_NATIVE
bool
@@ -331,7 +332,8 @@ config SWIOTLB

config HOTPLUG_CPU
bool "Support for enabling/disabling CPUs"
- depends on SMP && HOTPLUG && EXPERIMENTAL && (PPC_PSERIES || PPC_PMAC || PPC_POWERNV)
+ depends on SMP && HOTPLUG && EXPERIMENTAL && (PPC_PSERIES || \
+ PPC_PMAC || PPC_POWERNV || (PPC_85xx && !PPC_E500MC))
---help---
Say Y here to be able to disable and re-enable individual
CPUs at runtime on SMP machines.
diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
index ab9e402..94ec20a 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -30,6 +30,10 @@ extern void flush_dcache_page(struct page *page);
#define flush_dcache_mmap_lock(mapping) do { } while (0)
#define flush_dcache_mmap_unlock(mapping) do { } while (0)

+#if defined(CONFIG_FSL_BOOKE) || defined(CONFIG_6xx)
+extern void __flush_disable_L1(void);
+#endif
+
extern void __flush_icache_range(unsigned long, unsigned long);
static inline void flush_icache_range(unsigned long start, unsigned long stop)
{
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index ebc24dc..e807e9d 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -65,6 +65,7 @@ int generic_cpu_disable(void);
void generic_cpu_die(unsigned int cpu);
void generic_mach_cpu_die(void);
void generic_set_cpu_dead(unsigned int cpu);
+void generic_set_cpu_up(unsigned int cpu);
int generic_check_cpu_restart(unsigned int cpu);
#endif

@@ -190,6 +191,7 @@ extern unsigned long __secondary_hold_spinloop;
extern unsigned long __secondary_hold_acknowledge;
extern char __secondary_hold;

+extern void __early_start(void);
#endif /* __ASSEMBLY__ */

#endif /* __KERNEL__ */
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 28e6259..0cc6f02 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -1004,6 +1004,34 @@ _GLOBAL(flush_dcache_L1)

blr

+/* Flush L1 d-cache, invalidate and disable d-cache and i-cache */
+_GLOBAL(__flush_disable_L1)
+ mflr r10
+ bl flush_dcache_L1 /* Flush L1 d-cache */
+ mtlr r10
+
+ mfspr r4, SPRN_L1CSR0 /* Invalidate and disable d-cache */
+ li r5, 2
+ rlwimi r4, r5, 0, 3
+
+ msync
+ isync
+ mtspr SPRN_L1CSR0, r4
+ isync
+
+1: mfspr r4, SPRN_L1CSR0 /* Wait for the invalidate to finish */
+ andi. r4, r4, 2
+ bne 1b
+
+ mfspr r4, SPRN_L1CSR1 /* Invalidate and disable i-cache */
+ li r5, 2
+ rlwimi r4, r5, 0, 3
+
+ mtspr SPRN_L1CSR1, r4
+ isync
+
+ blr
+
#ifdef CONFIG_SMP
/* When we get here, r24 needs to hold the CPU # */
.globl __secondary_start
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index d9f9441..e0ffe03 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -423,6 +423,16 @@ void generic_set_cpu_dead(unsigned int cpu)
per_cpu(cpu_state, cpu) = CPU_DEAD;
}

+/*
+ * The cpu_state should be set to CPU_UP_PREPARE in kick_cpu(), otherwise
+ * the cpu_state is always CPU_DEAD after calling generic_set_cpu_dead(),
+ * which makes the delay in generic_cpu_die() not happen.
+ */
+void generic_set_cpu_up(unsigned int cpu)
+{
+ per_cpu(cpu_state, cpu) = CPU_UP_PREPARE;
+}
+
int generic_check_cpu_restart(unsigned int cpu)
{
return per_cpu(cpu_state, cpu) == CPU_UP_PREPARE;
diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
index 6862dda..6b95d4a 100644
--- a/arch/powerpc/platforms/85xx/smp.c
+++ b/arch/powerpc/platforms/85xx/smp.c
@@ -2,7 +2,7 @@
* Author: Andy Fleming <[email protected]>
* Kumar Gala <[email protected]>
*
- * Copyright 2006-2008, 2011 Freescale Semiconductor Inc.
+ * Copyright 2006-2008, 2011-2012 Freescale Semiconductor Inc.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
@@ -17,6 +17,7 @@
#include <linux/of.h>
#include <linux/kexec.h>
#include <linux/highmem.h>
+#include <linux/cpu.h>

#include <asm/machdep.h>
#include <asm/pgtable.h>
@@ -30,28 +31,55 @@
#include <sysdev/mpic.h>
#include "smp.h"

-extern void __early_start(void);
-
-#define BOOT_ENTRY_ADDR_UPPER 0
-#define BOOT_ENTRY_ADDR_LOWER 1
-#define BOOT_ENTRY_R3_UPPER 2
-#define BOOT_ENTRY_R3_LOWER 3
-#define BOOT_ENTRY_RESV 4
-#define BOOT_ENTRY_PIR 5
-#define BOOT_ENTRY_R6_UPPER 6
-#define BOOT_ENTRY_R6_LOWER 7
-#define NUM_BOOT_ENTRY 8
-#define SIZE_BOOT_ENTRY (NUM_BOOT_ENTRY * sizeof(u32))
-
-static int __init
-smp_85xx_kick_cpu(int nr)
+struct epapr_spin_table {
+ u32 addr_h;
+ u32 addr_l;
+ u32 r3_h;
+ u32 r3_l;
+ u32 reserved;
+ u32 pir;
+};
+
+static void __cpuinit smp_85xx_setup_cpu(int cpu_nr);
+
+#ifdef CONFIG_HOTPLUG_CPU
+static void __cpuinit smp_85xx_mach_cpu_die(void)
+{
+ unsigned int cpu = smp_processor_id();
+ u32 tmp;
+
+ local_irq_disable();
+ idle_task_exit();
+ generic_set_cpu_dead(cpu);
+ mb();
+
+ mtspr(SPRN_TCR, 0);
+
+ __flush_disable_L1();
+ tmp = (mfspr(SPRN_HID0) & ~(HID0_DOZE|HID0_SLEEP)) | HID0_NAP;
+ mtspr(SPRN_HID0, tmp);
+
+ /* Enter NAP mode. */
+ tmp = mfmsr();
+ tmp |= MSR_WE;
+ mb();
+ mtmsr(tmp);
+ isync();
+
+ while (1)
+ ;
+}
+#endif
+
+static int __cpuinit smp_85xx_kick_cpu(int nr)
{
unsigned long flags;
const u64 *cpu_rel_addr;
- __iomem u32 *bptr_vaddr;
+ __iomem struct epapr_spin_table *spin_table;
struct device_node *np;
- int n = 0, hw_cpu = get_hard_smp_processor_id(nr);
+ int hw_cpu = get_hard_smp_processor_id(nr);
int ioremappable;
+ int ret = 0;

WARN_ON(nr < 0 || nr >= NR_CPUS);
WARN_ON(hw_cpu < 0 || hw_cpu >= NR_CPUS);
@@ -76,49 +104,84 @@ smp_85xx_kick_cpu(int nr)

/* Map the spin table */
if (ioremappable)
- bptr_vaddr = ioremap(*cpu_rel_addr, SIZE_BOOT_ENTRY);
+ spin_table = ioremap(*cpu_rel_addr,
+ sizeof(struct epapr_spin_table));
else
- bptr_vaddr = phys_to_virt(*cpu_rel_addr);
+ spin_table = phys_to_virt(*cpu_rel_addr);

local_irq_save(flags);
-
- out_be32(bptr_vaddr + BOOT_ENTRY_PIR, hw_cpu);
#ifdef CONFIG_PPC32
- out_be32(bptr_vaddr + BOOT_ENTRY_ADDR_LOWER, __pa(__early_start));
+#ifdef CONFIG_HOTPLUG_CPU
+ /* Corresponding to generic_set_cpu_dead() */
+ generic_set_cpu_up(nr);
+
+ if (system_state == SYSTEM_RUNNING) {
+ out_be32(&spin_table->addr_l, 0);
+
+ /*
+ * We don't set the BPTR register here upon it points
+ * to the boot page properly.
+ */
+ mpic_reset_core(hw_cpu);
+
+ /* wait until core is ready... */
+ if (!spin_event_timeout(in_be32(&spin_table->addr_l) == 1,
+ 10000, 100)) {
+ pr_err("%s: timeout waiting for core %d to reset\n",
+ __func__, hw_cpu);
+ ret = -ENOENT;
+ goto out;
+ }
+
+ /* clear the acknowledge status */
+ __secondary_hold_acknowledge = -1;
+ }
+#endif
+ out_be32(&spin_table->pir, hw_cpu);
+ out_be32(&spin_table->addr_l, __pa(__early_start));

if (!ioremappable)
- flush_dcache_range((ulong)bptr_vaddr,
- (ulong)(bptr_vaddr + SIZE_BOOT_ENTRY));
+ flush_dcache_range((ulong)spin_table,
+ (ulong)spin_table + sizeof(struct epapr_spin_table));

/* Wait a bit for the CPU to ack. */
- while ((__secondary_hold_acknowledge != hw_cpu) && (++n < 1000))
- mdelay(1);
+ if (!spin_event_timeout(__secondary_hold_acknowledge == hw_cpu,
+ 10000, 100)) {
+ pr_err("%s: timeout waiting for core %d to ack\n",
+ __func__, hw_cpu);
+ ret = -ENOENT;
+ goto out;
+ }
+out:
#else
smp_generic_kick_cpu(nr);

- out_be64((u64 *)(bptr_vaddr + BOOT_ENTRY_ADDR_UPPER),
- __pa((u64)*((unsigned long long *) generic_secondary_smp_init)));
+ out_be32(&spin_table->pir, hw_cpu);
+ out_be64((u64 *)(&spin_table->addr_h),
+ __pa((u64)*((unsigned long long *)generic_secondary_smp_init)));

if (!ioremappable)
- flush_dcache_range((ulong)bptr_vaddr,
- (ulong)(bptr_vaddr + SIZE_BOOT_ENTRY));
+ flush_dcache_range((ulong)spin_table,
+ (ulong)spin_table + sizeof(struct epapr_spin_table));
#endif

local_irq_restore(flags);

if (ioremappable)
- iounmap(bptr_vaddr);
-
- pr_debug("waited %d msecs for CPU #%d.\n", n, nr);
+ iounmap(spin_table);

- return 0;
+ return ret;
}

struct smp_ops_t smp_85xx_ops = {
.kick_cpu = smp_85xx_kick_cpu,
+#ifdef CONFIG_HOTPLUG_CPU
+ .cpu_disable = generic_cpu_disable,
+ .cpu_die = generic_cpu_die,
+#endif
};

-#ifdef CONFIG_KEXEC
+#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU)
static struct ccsr_guts __iomem *guts;
static u64 timebase;
static int tb_req;
@@ -179,7 +242,9 @@ static void mpc85xx_take_timebase(void)

local_irq_restore(flags);
}
+#endif

+#ifdef CONFIG_KEXEC
atomic_t kexec_down_cpus = ATOMIC_INIT(0);

void mpc85xx_smp_kexec_cpu_down(int crash_shutdown, int secondary)
@@ -276,8 +341,7 @@ static void mpc85xx_smp_machine_kexec(struct kimage *image)
}
#endif /* CONFIG_KEXEC */

-static void __init
-smp_85xx_setup_cpu(int cpu_nr)
+static void __cpuinit smp_85xx_setup_cpu(int cpu_nr)
{
if (smp_85xx_ops.probe == smp_mpic_probe)
mpic_setup_this_cpu();
@@ -286,7 +350,7 @@ smp_85xx_setup_cpu(int cpu_nr)
doorbell_setup_this_cpu();
}

-#ifdef CONFIG_KEXEC
+#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU)
static const struct of_device_id guts_ids[] = {
{ .compatible = "fsl,mpc8572-guts", },
{ .compatible = "fsl,mpc8560-guts", },
@@ -321,10 +385,11 @@ void __init mpc85xx_smp_init(void)
smp_85xx_ops.cause_ipi = doorbell_cause_ipi;
}

-#ifdef CONFIG_KEXEC
+#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU)
np = of_find_matching_node(NULL, guts_ids);
if (np) {
guts = of_iomap(np, 0);
+ ppc_md.cpu_die = smp_85xx_mach_cpu_die;
smp_85xx_ops.give_timebase = mpc85xx_give_timebase;
smp_85xx_ops.take_timebase = mpc85xx_take_timebase;
of_node_put(np);
--
1.6.4.1

2012-05-11 11:54:44

by Chenhui Zhao

[permalink] [raw]
Subject: [PATCH v5 3/5] powerpc/85xx: add sleep and deep sleep support

From: Li Yang <[email protected]>

In sleep PM mode, the clocks of e500 core and unused IP blocks is
turned off. IP blocks which are allowed to wake up the processor
are still running.

Some Freescale chips like MPC8536 and P1022 has deep sleep PM mode
in addtion to the sleep PM mode.

While in deep sleep PM mode, additionally, the power supply is
removed from e500 core and most IP blocks. Only the blocks needed
to wake up the chip out of deep sleep are ON.

This patch supports 32-bit and 36-bit address space.

The sleep mode is equal to the Standby state in Linux. The deep sleep
mode is equal to the Suspend-to-RAM state of Linux Power Management.

Command to enter sleep mode.
echo standby > /sys/power/state
Command to enter deep sleep mode.
echo mem > /sys/power/state

Signed-off-by: Dave Liu <[email protected]>
Signed-off-by: Li Yang <[email protected]>
Signed-off-by: Jin Qing <[email protected]>
Signed-off-by: Jerry Huang <[email protected]>
Cc: Scott Wood <[email protected]>
Signed-off-by: Zhao Chenhui <[email protected]>
---
Changes for v5:
* Rename flush_disable_L1 to __flush_disable_L1.

arch/powerpc/Kconfig | 2 +-
arch/powerpc/include/asm/cacheflush.h | 5 +
arch/powerpc/kernel/Makefile | 3 +
arch/powerpc/kernel/l2cache_85xx.S | 53 +++
arch/powerpc/platforms/85xx/Makefile | 3 +
arch/powerpc/platforms/85xx/sleep.S | 609 +++++++++++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_pmc.c | 91 ++++-
arch/powerpc/sysdev/fsl_soc.h | 5 +
8 files changed, 752 insertions(+), 19 deletions(-)
create mode 100644 arch/powerpc/kernel/l2cache_85xx.S
create mode 100644 arch/powerpc/platforms/85xx/sleep.S

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d65ae35..039f0a6 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -673,7 +673,7 @@ config FSL_PCI
config FSL_PMC
bool
default y
- depends on SUSPEND && (PPC_85xx || PPC_86xx)
+ depends on SUSPEND && (PPC_85xx || PPC_86xx) && !PPC_E500MC
help
Freescale MPC85xx/MPC86xx power management controller support
(suspend/resume). For MPC83xx see platforms/83xx/suspend.c
diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
index 94ec20a..baa000c 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -33,6 +33,11 @@ extern void flush_dcache_page(struct page *page);
#if defined(CONFIG_FSL_BOOKE) || defined(CONFIG_6xx)
extern void __flush_disable_L1(void);
#endif
+#if defined(CONFIG_FSL_BOOKE)
+extern void flush_dcache_L1(void);
+#else
+#define flush_dcache_L1() do { } while (0)
+#endif

extern void __flush_icache_range(unsigned long, unsigned long);
static inline void flush_icache_range(unsigned long start, unsigned long stop)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index f5808a3..cb70dba 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -64,6 +64,9 @@ obj-$(CONFIG_FA_DUMP) += fadump.o
ifeq ($(CONFIG_PPC32),y)
obj-$(CONFIG_E500) += idle_e500.o
endif
+ifneq ($(CONFIG_PPC_E500MC),y)
+obj-$(CONFIG_PPC_85xx) += l2cache_85xx.o
+endif
obj-$(CONFIG_6xx) += idle_6xx.o l2cr_6xx.o cpu_setup_6xx.o
obj-$(CONFIG_TAU) += tau_6xx.o
obj-$(CONFIG_HIBERNATION) += swsusp.o suspend.o
diff --git a/arch/powerpc/kernel/l2cache_85xx.S b/arch/powerpc/kernel/l2cache_85xx.S
new file mode 100644
index 0000000..b0b7d1c
--- /dev/null
+++ b/arch/powerpc/kernel/l2cache_85xx.S
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2009-2012 Freescale Semiconductor, Inc. All rights reserved.
+ * Scott Wood <[email protected]>
+ * Dave Liu <[email protected]>
+ * implement the L2 cache operations of e500 based L2 controller
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <asm/reg.h>
+#include <asm/cputable.h>
+#include <asm/ppc_asm.h>
+#include <asm/asm-offsets.h>
+
+ .section .text
+
+ /* r3 = virtual address of L2 controller, WIMG = 01xx */
+_GLOBAL(flush_disable_L2)
+ /* It's a write-through cache, so only invalidation is needed. */
+ mbar
+ isync
+ lwz r4, 0(r3)
+ li r5, 1
+ rlwimi r4, r5, 30, 0xc0000000
+ stw r4, 0(r3)
+
+ /* Wait for the invalidate to finish */
+1: lwz r4, 0(r3)
+ andis. r4, r4, 0x4000
+ bne 1b
+ mbar
+
+ blr
+
+ /* r3 = virtual address of L2 controller, WIMG = 01xx */
+_GLOBAL(invalidate_enable_L2)
+ mbar
+ isync
+ lwz r4, 0(r3)
+ li r5, 3
+ rlwimi r4, r5, 30, 0xc0000000
+ stw r4, 0(r3)
+
+ /* Wait for the invalidate to finish */
+1: lwz r4, 0(r3)
+ andis. r4, r4, 0x4000
+ bne 1b
+ mbar
+
+ blr
diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
index 2125d4c..12b526a 100644
--- a/arch/powerpc/platforms/85xx/Makefile
+++ b/arch/powerpc/platforms/85xx/Makefile
@@ -2,6 +2,9 @@
# Makefile for the PowerPC 85xx linux kernel.
#
obj-$(CONFIG_SMP) += smp.o
+ifneq ($(CONFIG_PPC_E500MC),y)
+obj-$(CONFIG_SUSPEND) += sleep.o
+endif

obj-y += common.o

diff --git a/arch/powerpc/platforms/85xx/sleep.S b/arch/powerpc/platforms/85xx/sleep.S
new file mode 100644
index 0000000..b272f0c
--- /dev/null
+++ b/arch/powerpc/platforms/85xx/sleep.S
@@ -0,0 +1,609 @@
+/*
+ * Enter and leave deep sleep/sleep state on MPC85xx
+ *
+ * Author: Scott Wood <[email protected]>
+ *
+ * Copyright (C) 2006-2012 Freescale Semiconductor, Inc. All rights reserved.
+ *
+ * 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 <asm/page.h>
+#include <asm/ppc_asm.h>
+#include <asm/reg.h>
+#include <asm/asm-offsets.h>
+
+#define SS_TB 0x00
+#define SS_HID 0x08 /* 2 HIDs */
+#define SS_IAC 0x10 /* 2 IACs */
+#define SS_DAC 0x18 /* 2 DACs */
+#define SS_DBCR 0x20 /* 3 DBCRs */
+#define SS_PID 0x2c /* 3 PIDs */
+#define SS_SPRG 0x38 /* 8 SPRGs */
+#define SS_IVOR 0x58 /* 20 interrupt vectors */
+#define SS_TCR 0xa8
+#define SS_BUCSR 0xac
+#define SS_L1CSR 0xb0 /* 2 L1CSRs */
+#define SS_MSR 0xb8
+#define SS_USPRG 0xbc
+#define SS_GPREG 0xc0 /* r12-r31 */
+#define SS_LR 0x110
+#define SS_CR 0x114
+#define SS_SP 0x118
+#define SS_CURRENT 0x11c
+#define SS_IVPR 0x120
+#define SS_BPTR 0x124
+
+
+#define STATE_SAVE_SIZE 0x128
+
+ .section .data
+ .align 5
+mpc85xx_sleep_save_area:
+ .space STATE_SAVE_SIZE
+ccsrbase_low:
+ .long 0
+ccsrbase_high:
+ .long 0
+powmgtreq:
+ .long 0
+
+ .section .text
+ .align 12
+
+ /*
+ * r3 = high word of physical address of CCSR
+ * r4 = low word of physical address of CCSR
+ * r5 = JOG or deep sleep request
+ * JOG-0x00200000, deep sleep-0x00100000
+ */
+_GLOBAL(mpc85xx_enter_deep_sleep)
+ lis r6, ccsrbase_low@ha
+ stw r4, ccsrbase_low@l(r6)
+ lis r6, ccsrbase_high@ha
+ stw r3, ccsrbase_high@l(r6)
+
+ lis r6, powmgtreq@ha
+ stw r5, powmgtreq@l(r6)
+
+ lis r10, mpc85xx_sleep_save_area@h
+ ori r10, r10, mpc85xx_sleep_save_area@l
+
+ mfspr r5, SPRN_HID0
+ mfspr r6, SPRN_HID1
+
+ stw r5, SS_HID+0(r10)
+ stw r6, SS_HID+4(r10)
+
+ mfspr r4, SPRN_IAC1
+ mfspr r5, SPRN_IAC2
+ mfspr r6, SPRN_DAC1
+ mfspr r7, SPRN_DAC2
+
+ stw r4, SS_IAC+0(r10)
+ stw r5, SS_IAC+4(r10)
+ stw r6, SS_DAC+0(r10)
+ stw r7, SS_DAC+4(r10)
+
+ mfspr r4, SPRN_DBCR0
+ mfspr r5, SPRN_DBCR1
+ mfspr r6, SPRN_DBCR2
+
+ stw r4, SS_DBCR+0(r10)
+ stw r5, SS_DBCR+4(r10)
+ stw r6, SS_DBCR+8(r10)
+
+ mfspr r4, SPRN_PID0
+ mfspr r5, SPRN_PID1
+ mfspr r6, SPRN_PID2
+
+ stw r4, SS_PID+0(r10)
+ stw r5, SS_PID+4(r10)
+ stw r6, SS_PID+8(r10)
+
+ mfspr r4, SPRN_SPRG0
+ mfspr r5, SPRN_SPRG1
+ mfspr r6, SPRN_SPRG2
+ mfspr r7, SPRN_SPRG3
+
+ stw r4, SS_SPRG+0x00(r10)
+ stw r5, SS_SPRG+0x04(r10)
+ stw r6, SS_SPRG+0x08(r10)
+ stw r7, SS_SPRG+0x0c(r10)
+
+ mfspr r4, SPRN_SPRG4
+ mfspr r5, SPRN_SPRG5
+ mfspr r6, SPRN_SPRG6
+ mfspr r7, SPRN_SPRG7
+
+ stw r4, SS_SPRG+0x10(r10)
+ stw r5, SS_SPRG+0x14(r10)
+ stw r6, SS_SPRG+0x18(r10)
+ stw r7, SS_SPRG+0x1c(r10)
+
+ mfspr r4, SPRN_IVPR
+ stw r4, SS_IVPR(r10)
+
+ mfspr r4, SPRN_IVOR0
+ mfspr r5, SPRN_IVOR1
+ mfspr r6, SPRN_IVOR2
+ mfspr r7, SPRN_IVOR3
+
+ stw r4, SS_IVOR+0x00(r10)
+ stw r5, SS_IVOR+0x04(r10)
+ stw r6, SS_IVOR+0x08(r10)
+ stw r7, SS_IVOR+0x0c(r10)
+
+ mfspr r4, SPRN_IVOR4
+ mfspr r5, SPRN_IVOR5
+ mfspr r6, SPRN_IVOR6
+ mfspr r7, SPRN_IVOR7
+
+ stw r4, SS_IVOR+0x10(r10)
+ stw r5, SS_IVOR+0x14(r10)
+ stw r6, SS_IVOR+0x18(r10)
+ stw r7, SS_IVOR+0x1c(r10)
+
+ mfspr r4, SPRN_IVOR8
+ mfspr r5, SPRN_IVOR9
+ mfspr r6, SPRN_IVOR10
+ mfspr r7, SPRN_IVOR11
+
+ stw r4, SS_IVOR+0x20(r10)
+ stw r5, SS_IVOR+0x24(r10)
+ stw r6, SS_IVOR+0x28(r10)
+ stw r7, SS_IVOR+0x2c(r10)
+
+ mfspr r4, SPRN_IVOR12
+ mfspr r5, SPRN_IVOR13
+ mfspr r6, SPRN_IVOR14
+ mfspr r7, SPRN_IVOR15
+
+ stw r4, SS_IVOR+0x30(r10)
+ stw r5, SS_IVOR+0x34(r10)
+ stw r6, SS_IVOR+0x38(r10)
+ stw r7, SS_IVOR+0x3c(r10)
+
+ mfspr r4, SPRN_IVOR32
+ mfspr r5, SPRN_IVOR33
+ mfspr r6, SPRN_IVOR34
+ mfspr r7, SPRN_IVOR35
+
+ stw r4, SS_IVOR+0x40(r10)
+ stw r5, SS_IVOR+0x44(r10)
+ stw r6, SS_IVOR+0x48(r10)
+ stw r7, SS_IVOR+0x4c(r10)
+
+ mfspr r4, SPRN_TCR
+ mfspr r5, SPRN_BUCSR
+ mfspr r6, SPRN_L1CSR0
+ mfspr r7, SPRN_L1CSR1
+ mfspr r8, SPRN_USPRG0
+
+ stw r4, SS_TCR(r10)
+ stw r5, SS_BUCSR(r10)
+ stw r6, SS_L1CSR+0(r10)
+ stw r7, SS_L1CSR+4(r10)
+ stw r8, SS_USPRG+0(r10)
+
+ stmw r12, SS_GPREG(r10)
+
+ mfmsr r4
+ mflr r5
+ mfcr r6
+
+ stw r4, SS_MSR(r10)
+ stw r5, SS_LR(r10)
+ stw r6, SS_CR(r10)
+ stw r1, SS_SP(r10)
+ stw r2, SS_CURRENT(r10)
+
+1: mftbu r4
+ mftb r5
+ mftbu r6
+ cmpw r4, r6
+ bne 1b
+
+ stw r4, SS_TB+0(r10)
+ stw r5, SS_TB+4(r10)
+
+ lis r5, ccsrbase_low@ha
+ lwz r4, ccsrbase_low@l(r5)
+ lis r5, ccsrbase_high@ha
+ lwz r3, ccsrbase_high@l(r5)
+
+ /* Disable machine checks and critical exceptions */
+ mfmsr r5
+ rlwinm r5, r5, 0, ~MSR_CE
+ rlwinm r5, r5, 0, ~MSR_ME
+ mtmsr r5
+ isync
+
+ /* Use TLB1[15] to map the CCSR at 0xf0000000 */
+ lis r5, 0x100f
+ mtspr SPRN_MAS0, r5
+ lis r5, 0xc000
+ ori r5, r5, 0x0500
+ mtspr SPRN_MAS1, r5
+ lis r5, 0xf000
+ ori r5, r5, 0x000a
+ mtspr SPRN_MAS2, r5
+ rlwinm r5, r4, 0, 0xfffff000
+ ori r5, r5, 0x0005
+ mtspr SPRN_MAS3, r5
+ mtspr SPRN_MAS7, r3
+ isync
+ tlbwe
+ isync
+
+ lis r3, 0xf000
+ lwz r4, 0x20(r3)
+ stw r4, SS_BPTR(r10)
+
+ lis r3, 0xf002 /* L2 cache controller at CCSR+0x20000 */
+ bl flush_disable_L2
+ bl __flush_disable_L1
+
+ /* Enable I-cache, so as not to upset the bus
+ * with our loop.
+ */
+
+ mfspr r4, SPRN_L1CSR1
+ ori r4, r4, 1
+ mtspr SPRN_L1CSR1, r4
+ isync
+
+ /* Set boot page translation */
+ lis r3, 0xf000
+ lis r4, (mpc85xx_deep_resume - PAGE_OFFSET)@h
+ ori r4, r4, (mpc85xx_deep_resume - PAGE_OFFSET)@l
+ rlwinm r4, r4, 20, 0x000fffff
+ oris r4, r4, 0x8000
+ stw r4, 0x20(r3)
+ lwz r4, 0x20(r3) /* read-back to flush write */
+ twi 0, r4, 0
+ isync
+
+ /* Disable the decrementer */
+ mfspr r4, SPRN_TCR
+ rlwinm r4, r4, 0, ~TCR_DIE
+ mtspr SPRN_TCR, r4
+
+ mfspr r4, SPRN_TSR
+ oris r4, r4, TSR_DIS@h
+ mtspr SPRN_TSR, r4
+
+ /* set PMRCCR[VRCNT] to wait power stable for 40ms */
+ lis r3, 0xf00e
+ lwz r4, 0x84(r3)
+ clrlwi r4, r4, 16
+ oris r4, r4, 0x12a3
+ stw r4, 0x84(r3)
+ lwz r4, 0x84(r3)
+
+ /* set deep sleep bit in POWMGTSCR */
+ lis r3, powmgtreq@ha
+ lwz r8, powmgtreq@l(r3)
+
+ lis r3, 0xf00e
+ lwz r4, 0x80(r3)
+ or r4, r4, r8
+ stw r4, 0x80(r3)
+ lwz r4, 0x80(r3) /* read-back to flush write */
+ twi 0, r4, 0
+ isync
+
+ mftb r5
+1: /* spin until either we enter deep sleep, or the sleep process is
+ * aborted due to a pending wakeup event. Wait some time between
+ * accesses, so we don't flood the bus and prevent the pmc from
+ * detecting an idle system.
+ */
+
+ mftb r4
+ subf r7, r5, r4
+ cmpwi r7, 1000
+ blt 1b
+ mr r5, r4
+
+ lwz r6, 0x80(r3)
+ andis. r6, r6, 0x0010
+ bne 1b
+ b 2f
+
+2: mfspr r4, SPRN_PIR
+ andi. r4, r4, 1
+99: bne 99b
+
+ /* Establish a temporary 64MB 0->0 mapping in TLB1[1]. */
+ lis r4, 0x1001
+ mtspr SPRN_MAS0, r4
+ lis r4, 0xc000
+ ori r4, r4, 0x0800
+ mtspr SPRN_MAS1, r4
+ li r4, 0
+ mtspr SPRN_MAS2, r4
+ li r4, 0x0015
+ mtspr SPRN_MAS3, r4
+ li r4, 0
+ mtspr SPRN_MAS7, r4
+ isync
+ tlbwe
+ isync
+
+ lis r3, (3f - PAGE_OFFSET)@h
+ ori r3, r3, (3f - PAGE_OFFSET)@l
+ mtctr r3
+ bctr
+
+ /* Locate the resume vector in the last word of the current page. */
+ . = mpc85xx_enter_deep_sleep + 0xffc
+mpc85xx_deep_resume:
+ b 2b
+
+3:
+ /* Restore the contents of TLB1[0]. It is assumed that it covers
+ * the currently executing code and the sleep save area, and that
+ * it does not alias our temporary mapping (which is at virtual zero).
+ */
+ lis r3, (TLBCAM - PAGE_OFFSET)@h
+ ori r3, r3, (TLBCAM - PAGE_OFFSET)@l
+
+ lwz r4, 0(r3)
+ lwz r5, 4(r3)
+ lwz r6, 8(r3)
+ lwz r7, 12(r3)
+ lwz r8, 16(r3)
+
+ mtspr SPRN_MAS0, r4
+ mtspr SPRN_MAS1, r5
+ mtspr SPRN_MAS2, r6
+ mtspr SPRN_MAS3, r7
+ mtspr SPRN_MAS7, r8
+
+ isync
+ tlbwe
+ isync
+
+ /* Access the ccsrbase address with TLB1[0] */
+ lis r5, ccsrbase_low@ha
+ lwz r4, ccsrbase_low@l(r5)
+ lis r5, ccsrbase_high@ha
+ lwz r3, ccsrbase_high@l(r5)
+
+ /* Use TLB1[15] to map the CCSR at 0xf0000000 */
+ lis r5, 0x100f
+ mtspr SPRN_MAS0, r5
+ lis r5, 0xc000
+ ori r5, r5, 0x0500
+ mtspr SPRN_MAS1, r5
+ lis r5, 0xf000
+ ori r5, r5, 0x000a
+ mtspr SPRN_MAS2, r5
+ rlwinm r5, r4, 0, 0xfffff000
+ ori r5, r5, 0x0005
+ mtspr SPRN_MAS3, r5
+ mtspr SPRN_MAS7, r3
+ isync
+ tlbwe
+ isync
+
+ lis r3, 0xf002 /* L2 cache controller at CCSR+0x20000 */
+ bl invalidate_enable_L2
+
+ /* Access the MEM(r10) with TLB1[0] */
+ lis r10, mpc85xx_sleep_save_area@h
+ ori r10, r10, mpc85xx_sleep_save_area@l
+
+ lis r3, 0xf000
+ lwz r4, SS_BPTR(r10)
+ stw r4, 0x20(r3) /* restore BPTR */
+
+ /* Program shift running space to PAGE_OFFSET */
+ mfmsr r3
+ lis r4, 1f@h
+ ori r4, r4, 1f@l
+
+ mtsrr1 r3
+ mtsrr0 r4
+ rfi
+
+1: /* Restore the rest of TLB1, in ascending order so that
+ * the TLB1[1] gets invalidated first.
+ *
+ * XXX: It's better to invalidate the temporary mapping
+ * TLB1[15] for CCSR before restore any TLB1 entry include 0.
+ */
+ lis r4, 0x100f
+ mtspr SPRN_MAS0, r4
+ lis r4, 0
+ mtspr SPRN_MAS1, r4
+ isync
+ tlbwe
+ isync
+
+ lis r3, (TLBCAM + 5*4 - 4)@h
+ ori r3, r3, (TLBCAM + 5*4 - 4)@l
+ li r4, 15
+ mtctr r4
+
+2:
+ lwz r5, 4(r3)
+ lwz r6, 8(r3)
+ lwz r7, 12(r3)
+ lwz r8, 16(r3)
+ lwzu r9, 20(r3)
+
+ mtspr SPRN_MAS0, r5
+ mtspr SPRN_MAS1, r6
+ mtspr SPRN_MAS2, r7
+ mtspr SPRN_MAS3, r8
+ mtspr SPRN_MAS7, r9
+
+ isync
+ tlbwe
+ isync
+ bdnz 2b
+
+ lis r10, mpc85xx_sleep_save_area@h
+ ori r10, r10, mpc85xx_sleep_save_area@l
+
+ lwz r5, SS_HID+0(r10)
+ lwz r6, SS_HID+4(r10)
+
+ isync
+ mtspr SPRN_HID0, r5
+ isync
+
+ msync
+ mtspr SPRN_HID1, r6
+ isync
+
+ lwz r4, SS_IAC+0(r10)
+ lwz r5, SS_IAC+4(r10)
+ lwz r6, SS_DAC+0(r10)
+ lwz r7, SS_DAC+4(r10)
+
+ mtspr SPRN_IAC1, r4
+ mtspr SPRN_IAC2, r5
+ mtspr SPRN_DAC1, r6
+ mtspr SPRN_DAC2, r7
+
+ lwz r4, SS_DBCR+0(r10)
+ lwz r5, SS_DBCR+4(r10)
+ lwz r6, SS_DBCR+8(r10)
+
+ mtspr SPRN_DBCR0, r4
+ mtspr SPRN_DBCR1, r5
+ mtspr SPRN_DBCR2, r6
+
+ lwz r4, SS_PID+0(r10)
+ lwz r5, SS_PID+4(r10)
+ lwz r6, SS_PID+8(r10)
+
+ mtspr SPRN_PID0, r4
+ mtspr SPRN_PID1, r5
+ mtspr SPRN_PID2, r6
+
+ lwz r4, SS_SPRG+0x00(r10)
+ lwz r5, SS_SPRG+0x04(r10)
+ lwz r6, SS_SPRG+0x08(r10)
+ lwz r7, SS_SPRG+0x0c(r10)
+
+ mtspr SPRN_SPRG0, r4
+ mtspr SPRN_SPRG1, r5
+ mtspr SPRN_SPRG2, r6
+ mtspr SPRN_SPRG3, r7
+
+ lwz r4, SS_SPRG+0x10(r10)
+ lwz r5, SS_SPRG+0x14(r10)
+ lwz r6, SS_SPRG+0x18(r10)
+ lwz r7, SS_SPRG+0x1c(r10)
+
+ mtspr SPRN_SPRG4, r4
+ mtspr SPRN_SPRG5, r5
+ mtspr SPRN_SPRG6, r6
+ mtspr SPRN_SPRG7, r7
+
+ lwz r4, SS_IVPR(r10)
+ mtspr SPRN_IVPR, r4
+
+ lwz r4, SS_IVOR+0x00(r10)
+ lwz r5, SS_IVOR+0x04(r10)
+ lwz r6, SS_IVOR+0x08(r10)
+ lwz r7, SS_IVOR+0x0c(r10)
+
+ mtspr SPRN_IVOR0, r4
+ mtspr SPRN_IVOR1, r5
+ mtspr SPRN_IVOR2, r6
+ mtspr SPRN_IVOR3, r7
+
+ lwz r4, SS_IVOR+0x10(r10)
+ lwz r5, SS_IVOR+0x14(r10)
+ lwz r6, SS_IVOR+0x18(r10)
+ lwz r7, SS_IVOR+0x1c(r10)
+
+ mtspr SPRN_IVOR4, r4
+ mtspr SPRN_IVOR5, r5
+ mtspr SPRN_IVOR6, r6
+ mtspr SPRN_IVOR7, r7
+
+ lwz r4, SS_IVOR+0x20(r10)
+ lwz r5, SS_IVOR+0x24(r10)
+ lwz r6, SS_IVOR+0x28(r10)
+ lwz r7, SS_IVOR+0x2c(r10)
+
+ mtspr SPRN_IVOR8, r4
+ mtspr SPRN_IVOR9, r5
+ mtspr SPRN_IVOR10, r6
+ mtspr SPRN_IVOR11, r7
+
+ lwz r4, SS_IVOR+0x30(r10)
+ lwz r5, SS_IVOR+0x34(r10)
+ lwz r6, SS_IVOR+0x38(r10)
+ lwz r7, SS_IVOR+0x3c(r10)
+
+ mtspr SPRN_IVOR12, r4
+ mtspr SPRN_IVOR13, r5
+ mtspr SPRN_IVOR14, r6
+ mtspr SPRN_IVOR15, r7
+
+ lwz r4, SS_IVOR+0x40(r10)
+ lwz r5, SS_IVOR+0x44(r10)
+ lwz r6, SS_IVOR+0x48(r10)
+ lwz r7, SS_IVOR+0x4c(r10)
+
+ mtspr SPRN_IVOR32, r4
+ mtspr SPRN_IVOR33, r5
+ mtspr SPRN_IVOR34, r6
+ mtspr SPRN_IVOR35, r7
+
+ lwz r4, SS_TCR(r10)
+ lwz r5, SS_BUCSR(r10)
+ lwz r6, SS_L1CSR+0(r10)
+ lwz r7, SS_L1CSR+4(r10)
+ lwz r8, SS_USPRG+0(r10)
+
+ mtspr SPRN_TCR, r4
+ mtspr SPRN_BUCSR, r5
+
+ msync
+ isync
+ mtspr SPRN_L1CSR0, r6
+ isync
+
+ mtspr SPRN_L1CSR1, r7
+ isync
+
+ mtspr SPRN_USPRG0, r8
+
+ lmw r12, SS_GPREG(r10)
+
+ lwz r1, SS_SP(r10)
+ lwz r2, SS_CURRENT(r10)
+ lwz r4, SS_MSR(r10)
+ lwz r5, SS_LR(r10)
+ lwz r6, SS_CR(r10)
+
+ msync
+ mtmsr r4
+ isync
+
+ mtlr r5
+ mtcr r6
+
+ li r4, 0
+ mtspr SPRN_TBWL, r4
+
+ lwz r4, SS_TB+0(r10)
+ lwz r5, SS_TB+4(r10)
+
+ mtspr SPRN_TBWU, r4
+ mtspr SPRN_TBWL, r5
+
+ lis r3, 1
+ mtdec r3
+
+ blr
diff --git a/arch/powerpc/sysdev/fsl_pmc.c b/arch/powerpc/sysdev/fsl_pmc.c
index 592a0f8..1dc6e9e 100644
--- a/arch/powerpc/sysdev/fsl_pmc.c
+++ b/arch/powerpc/sysdev/fsl_pmc.c
@@ -2,6 +2,7 @@
* Suspend/resume support
*
* Copyright 2009 MontaVista Software, Inc.
+ * Copyright 2010-2012 Freescale Semiconductor Inc.
*
* Author: Anton Vorontsov <[email protected]>
*
@@ -19,39 +20,83 @@
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/of_platform.h>
+#include <linux/pm.h>
+#include <asm/cacheflush.h>
+#include <asm/switch_to.h>
+
+#include <sysdev/fsl_soc.h>

struct pmc_regs {
__be32 devdisr;
__be32 devdisr2;
- __be32 :32;
- __be32 :32;
- __be32 pmcsr;
-#define PMCSR_SLP (1 << 17)
+ __be32 res1;
+ __be32 res2;
+ __be32 powmgtcsr;
+#define POWMGTCSR_SLP 0x00020000
+#define POWMGTCSR_DPSLP 0x00100000
+ __be32 res3[2];
+ __be32 pmcdr;
};

-static struct device *pmc_dev;
static struct pmc_regs __iomem *pmc_regs;
+static unsigned int pmc_flag;
+
+#define PMC_SLEEP 0x1
+#define PMC_DEEP_SLEEP 0x2

static int pmc_suspend_enter(suspend_state_t state)
{
- int ret;
+ int ret = 0;
+
+ switch (state) {
+#ifdef CONFIG_PPC_85xx
+ case PM_SUSPEND_MEM:
+#ifdef CONFIG_SPE
+ enable_kernel_spe();
+#endif
+ enable_kernel_fp();
+
+ pr_debug("%s: Entering deep sleep\n", __func__);
+
+ local_irq_disable();
+ mpc85xx_enter_deep_sleep(get_immrbase(), POWMGTCSR_DPSLP);
+
+ pr_debug("%s: Resumed from deep sleep\n", __func__);
+ break;
+#endif
+
+ case PM_SUSPEND_STANDBY:
+ local_irq_disable();
+ flush_dcache_L1();

- setbits32(&pmc_regs->pmcsr, PMCSR_SLP);
- /* At this point, the CPU is asleep. */
+ setbits32(&pmc_regs->powmgtcsr, POWMGTCSR_SLP);
+ /* At this point, the CPU is asleep. */

- /* Upon resume, wait for SLP bit to be clear. */
- ret = spin_event_timeout((in_be32(&pmc_regs->pmcsr) & PMCSR_SLP) == 0,
- 10000, 10) ? 0 : -ETIMEDOUT;
- if (ret)
- dev_err(pmc_dev, "tired waiting for SLP bit to clear\n");
+ /* Upon resume, wait for SLP bit to be clear. */
+ ret = spin_event_timeout(
+ (in_be32(&pmc_regs->powmgtcsr) & POWMGTCSR_SLP) == 0,
+ 10000, 10);
+ if (!ret) {
+ pr_err("%s: timeout waiting for SLP bit "
+ "to be cleared\n", __func__);
+ ret = -EINVAL;
+ }
+ break;
+
+ default:
+ ret = -EINVAL;
+
+ }
return ret;
}

static int pmc_suspend_valid(suspend_state_t state)
{
- if (state != PM_SUSPEND_STANDBY)
+ if (((pmc_flag & PMC_SLEEP) && (state == PM_SUSPEND_STANDBY)) ||
+ ((pmc_flag & PMC_DEEP_SLEEP) && (state == PM_SUSPEND_MEM)))
+ return 1;
+ else
return 0;
- return 1;
}

static const struct platform_suspend_ops pmc_suspend_ops = {
@@ -59,14 +104,24 @@ static const struct platform_suspend_ops pmc_suspend_ops = {
.enter = pmc_suspend_enter,
};

-static int pmc_probe(struct platform_device *ofdev)
+static int pmc_probe(struct platform_device *pdev)
{
- pmc_regs = of_iomap(ofdev->dev.of_node, 0);
+ struct device_node *np = pdev->dev.of_node;
+
+ pmc_regs = of_iomap(np, 0);
if (!pmc_regs)
return -ENOMEM;

- pmc_dev = &ofdev->dev;
+ pmc_flag = PMC_SLEEP;
+ if (of_device_is_compatible(np, "fsl,mpc8536-pmc"))
+ pmc_flag |= PMC_DEEP_SLEEP;
+
+ if (of_device_is_compatible(np, "fsl,p1022-pmc"))
+ pmc_flag |= PMC_DEEP_SLEEP;
+
suspend_set_ops(&pmc_suspend_ops);
+
+ pr_info("Freescale PMC driver\n");
return 0;
}

diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
index c6d0073..949377d 100644
--- a/arch/powerpc/sysdev/fsl_soc.h
+++ b/arch/powerpc/sysdev/fsl_soc.h
@@ -48,5 +48,10 @@ extern struct platform_diu_data_ops diu_ops;
void fsl_hv_restart(char *cmd);
void fsl_hv_halt(void);

+/*
+ * Cast the ccsrbar to 64-bit parameter so that the assembly
+ * code can be compatible with both 32-bit & 36-bit.
+ */
+extern void mpc85xx_enter_deep_sleep(u64 ccsrbar, u32 powmgtreq);
#endif
#endif
--
1.6.4.1

2012-05-11 11:54:40

by Chenhui Zhao

[permalink] [raw]
Subject: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source

Add APIs for setting wakeup source and lossless Ethernet in low power modes.
These APIs can be used by wake-on-packet feature.

Signed-off-by: Dave Liu <[email protected]>
Signed-off-by: Li Yang <[email protected]>
Signed-off-by: Jin Qing <[email protected]>
Signed-off-by: Zhao Chenhui <[email protected]>
---
arch/powerpc/sysdev/fsl_pmc.c | 71 ++++++++++++++++++++++++++++++++++++++++-
arch/powerpc/sysdev/fsl_soc.h | 9 +++++
2 files changed, 79 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_pmc.c b/arch/powerpc/sysdev/fsl_pmc.c
index 1dc6e9e..c1170f7 100644
--- a/arch/powerpc/sysdev/fsl_pmc.c
+++ b/arch/powerpc/sysdev/fsl_pmc.c
@@ -34,6 +34,7 @@ struct pmc_regs {
__be32 powmgtcsr;
#define POWMGTCSR_SLP 0x00020000
#define POWMGTCSR_DPSLP 0x00100000
+#define POWMGTCSR_LOSSLESS 0x00400000
__be32 res3[2];
__be32 pmcdr;
};
@@ -43,6 +44,74 @@ static unsigned int pmc_flag;

#define PMC_SLEEP 0x1
#define PMC_DEEP_SLEEP 0x2
+#define PMC_LOSSLESS 0x4
+
+/**
+ * mpc85xx_pmc_set_wake - enable devices as wakeup event source
+ * @pdev: platform device affected
+ * @enable: True to enable event generation; false to disable
+ *
+ * This enables the device as a wakeup event source, or disables it.
+ *
+ * RETURN VALUE:
+ * 0 is returned on success
+ * -EINVAL is returned if device is not supposed to wake up the system
+ * Error code depending on the platform is returned if both the platform and
+ * the native mechanism fail to enable the generation of wake-up events
+ */
+int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool enable)
+{
+ int ret = 0;
+ struct device_node *clk_np;
+ u32 *prop;
+ u32 pmcdr_mask;
+
+ if (!pmc_regs) {
+ pr_err("%s: PMC is unavailable\n", __func__);
+ return -ENODEV;
+ }
+
+ if (enable && !device_may_wakeup(&pdev->dev))
+ return -EINVAL;
+
+ clk_np = of_parse_phandle(pdev->dev.of_node, "fsl,pmc-handle", 0);
+ if (!clk_np)
+ return -EINVAL;
+
+ prop = (u32 *)of_get_property(clk_np, "fsl,pmcdr-mask", NULL);
+ if (!prop) {
+ ret = -EINVAL;
+ goto out;
+ }
+ pmcdr_mask = be32_to_cpup(prop);
+
+ if (enable)
+ /* clear to enable clock in low power mode */
+ clrbits32(&pmc_regs->pmcdr, pmcdr_mask);
+ else
+ setbits32(&pmc_regs->pmcdr, pmcdr_mask);
+
+out:
+ of_node_put(clk_np);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(mpc85xx_pmc_set_wake);
+
+/**
+ * mpc85xx_pmc_set_lossless_ethernet - enable lossless ethernet
+ * in (deep) sleep mode
+ * @enable: True to enable event generation; false to disable
+ */
+void mpc85xx_pmc_set_lossless_ethernet(int enable)
+{
+ if (pmc_flag & PMC_LOSSLESS) {
+ if (enable)
+ setbits32(&pmc_regs->powmgtcsr, POWMGTCSR_LOSSLESS);
+ else
+ clrbits32(&pmc_regs->powmgtcsr, POWMGTCSR_LOSSLESS);
+ }
+}
+EXPORT_SYMBOL_GPL(mpc85xx_pmc_set_lossless_ethernet);

static int pmc_suspend_enter(suspend_state_t state)
{
@@ -117,7 +186,7 @@ static int pmc_probe(struct platform_device *pdev)
pmc_flag |= PMC_DEEP_SLEEP;

if (of_device_is_compatible(np, "fsl,p1022-pmc"))
- pmc_flag |= PMC_DEEP_SLEEP;
+ pmc_flag |= PMC_DEEP_SLEEP | PMC_LOSSLESS;

suspend_set_ops(&pmc_suspend_ops);

diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
index 949377d..8976534 100644
--- a/arch/powerpc/sysdev/fsl_soc.h
+++ b/arch/powerpc/sysdev/fsl_soc.h
@@ -3,6 +3,7 @@
#ifdef __KERNEL__

#include <asm/mmu.h>
+#include <linux/platform_device.h>

struct spi_device;

@@ -21,6 +22,14 @@ struct device_node;

extern void fsl_rstcr_restart(char *cmd);

+#ifdef CONFIG_FSL_PMC
+extern int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool enable);
+extern void mpc85xx_pmc_set_lossless_ethernet(int enable);
+#else
+#define mpc85xx_pmc_set_wake(pdev, enable) do { } while (0)
+#define mpc85xx_pmc_set_lossless_ethernet(enable) do { } while (0)
+#endif
+
#if defined(CONFIG_FB_FSL_DIU) || defined(CONFIG_FB_FSL_DIU_MODULE)

/* The different ports that the DIU can be connected to */
--
1.6.4.1

2012-05-29 07:30:51

by Li Yang

[permalink] [raw]
Subject: Re: [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync

Hi Scott,

Thanks for the valuable comment raised before and we have updated the
patches accordingly. Please review the updated patch set and ACK if
they are good to you. We hope it can be applied in this window.

Leo

On Fri, May 11, 2012 at 7:53 PM, Zhao Chenhui
<[email protected]> wrote:
> Do hardware timebase sync. Firstly, stop all timebases, and transfer
> the timebase value of the boot core to the other core. Finally,
> start all timebases.
>
> Only apply to dual-core chips, such as MPC8572, P2020, etc.
>
> Signed-off-by: Zhao Chenhui <[email protected]>
> Signed-off-by: Li Yang <[email protected]>
> ---
>  arch/powerpc/include/asm/fsl_guts.h |    2 +
>  arch/powerpc/platforms/85xx/smp.c   |   93 +++++++++++++++++++++++++++++++++--
>  2 files changed, 91 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/fsl_guts.h b/arch/powerpc/include/asm/fsl_guts.h
> index aa4c488..dd5ba2c 100644
> --- a/arch/powerpc/include/asm/fsl_guts.h
> +++ b/arch/powerpc/include/asm/fsl_guts.h
> @@ -48,6 +48,8 @@ struct ccsr_guts {
>         __be32  dmuxcr;                /* 0x.0068 - DMA Mux Control Register */
>         u8     res06c[0x70 - 0x6c];
>        __be32  devdisr;        /* 0x.0070 - Device Disable Control */
> +#define CCSR_GUTS_DEVDISR_TB1  0x00001000
> +#define CCSR_GUTS_DEVDISR_TB0  0x00004000
>        __be32  devdisr2;       /* 0x.0074 - Device Disable Control 2 */
>        u8      res078[0x7c - 0x78];
>        __be32  pmjcr;          /* 0x.007c - 4 Power Management Jog Control Register */
> diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
> index ff42490..6862dda 100644
> --- a/arch/powerpc/platforms/85xx/smp.c
> +++ b/arch/powerpc/platforms/85xx/smp.c
> @@ -24,6 +24,7 @@
>  #include <asm/mpic.h>
>  #include <asm/cacheflush.h>
>  #include <asm/dbell.h>
> +#include <asm/fsl_guts.h>
>
>  #include <sysdev/fsl_soc.h>
>  #include <sysdev/mpic.h>
> @@ -115,13 +116,70 @@ smp_85xx_kick_cpu(int nr)
>
>  struct smp_ops_t smp_85xx_ops = {
>        .kick_cpu = smp_85xx_kick_cpu,
> -#ifdef CONFIG_KEXEC
> -       .give_timebase  = smp_generic_give_timebase,
> -       .take_timebase  = smp_generic_take_timebase,
> -#endif
>  };
>
>  #ifdef CONFIG_KEXEC
> +static struct ccsr_guts __iomem *guts;
> +static u64 timebase;
> +static int tb_req;
> +static int tb_valid;
> +
> +static void mpc85xx_timebase_freeze(int freeze)
> +{
> +       unsigned int mask;
> +
> +       if (!guts)
> +               return;
> +
> +       mask = CCSR_GUTS_DEVDISR_TB0 | CCSR_GUTS_DEVDISR_TB1;
> +       if (freeze)
> +               setbits32(&guts->devdisr, mask);
> +       else
> +               clrbits32(&guts->devdisr, mask);
> +
> +       in_be32(&guts->devdisr);
> +}
> +
> +static void mpc85xx_give_timebase(void)
> +{
> +       unsigned long flags;
> +
> +       local_irq_save(flags);
> +
> +       while (!tb_req)
> +               barrier();
> +       tb_req = 0;
> +
> +       mpc85xx_timebase_freeze(1);
> +       timebase = get_tb();
> +       mb();
> +       tb_valid = 1;
> +
> +       while (tb_valid)
> +               barrier();
> +
> +       mpc85xx_timebase_freeze(0);
> +
> +       local_irq_restore(flags);
> +}
> +
> +static void mpc85xx_take_timebase(void)
> +{
> +       unsigned long flags;
> +
> +       local_irq_save(flags);
> +
> +       tb_req = 1;
> +       while (!tb_valid)
> +               barrier();
> +
> +       set_tb(timebase >> 32, timebase & 0xffffffff);
> +       mb();
> +       tb_valid = 0;
> +
> +       local_irq_restore(flags);
> +}
> +
>  atomic_t kexec_down_cpus = ATOMIC_INIT(0);
>
>  void mpc85xx_smp_kexec_cpu_down(int crash_shutdown, int secondary)
> @@ -228,6 +286,20 @@ smp_85xx_setup_cpu(int cpu_nr)
>                doorbell_setup_this_cpu();
>  }
>
> +#ifdef CONFIG_KEXEC
> +static const struct of_device_id guts_ids[] = {
> +       { .compatible = "fsl,mpc8572-guts", },
> +       { .compatible = "fsl,mpc8560-guts", },
> +       { .compatible = "fsl,mpc8536-guts", },
> +       { .compatible = "fsl,p1020-guts", },
> +       { .compatible = "fsl,p1021-guts", },
> +       { .compatible = "fsl,p1022-guts", },
> +       { .compatible = "fsl,p1023-guts", },
> +       { .compatible = "fsl,p2020-guts", },
> +       {},
> +};
> +#endif
> +
>  void __init mpc85xx_smp_init(void)
>  {
>        struct device_node *np;
> @@ -249,6 +321,19 @@ void __init mpc85xx_smp_init(void)
>                smp_85xx_ops.cause_ipi = doorbell_cause_ipi;
>        }
>
> +#ifdef CONFIG_KEXEC
> +       np = of_find_matching_node(NULL, guts_ids);
> +       if (np) {
> +               guts = of_iomap(np, 0);
> +               smp_85xx_ops.give_timebase = mpc85xx_give_timebase;
> +               smp_85xx_ops.take_timebase = mpc85xx_take_timebase;
> +               of_node_put(np);
> +       } else {
> +               smp_85xx_ops.give_timebase = smp_generic_give_timebase;
> +               smp_85xx_ops.take_timebase = smp_generic_take_timebase;
> +       }
> +#endif
> +
>        smp_ops = &smp_85xx_ops;
>
>  #ifdef CONFIG_KEXEC
> --
> 1.6.4.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



--
- Leo

2012-06-01 15:40:15

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync

On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> #ifdef CONFIG_KEXEC
> +static struct ccsr_guts __iomem *guts;
> +static u64 timebase;
> +static int tb_req;
> +static int tb_valid;
> +
> +static void mpc85xx_timebase_freeze(int freeze)

Why is this under CONFIG_KEXEC? It'll also be needed for CPU hotplug.

> +{
> + unsigned int mask;
> +
> + if (!guts)
> + return;
> +
> + mask = CCSR_GUTS_DEVDISR_TB0 | CCSR_GUTS_DEVDISR_TB1;
> + if (freeze)
> + setbits32(&guts->devdisr, mask);
> + else
> + clrbits32(&guts->devdisr, mask);
> +
> + in_be32(&guts->devdisr);
> +}
> +
> +static void mpc85xx_give_timebase(void)
> +{
> + unsigned long flags;
> +
> + local_irq_save(flags);
> +
> + while (!tb_req)
> + barrier();
> + tb_req = 0;
> +
> + mpc85xx_timebase_freeze(1);
> + timebase = get_tb();
> + mb();
> + tb_valid = 1;
> +
> + while (tb_valid)
> + barrier();
> +
> + mpc85xx_timebase_freeze(0);
> +
> + local_irq_restore(flags);
> +}
> +
> +static void mpc85xx_take_timebase(void)
> +{
> + unsigned long flags;
> +
> + local_irq_save(flags);
> +
> + tb_req = 1;
> + while (!tb_valid)
> + barrier();
> +
> + set_tb(timebase >> 32, timebase & 0xffffffff);
> + mb();
> + tb_valid = 0;
> +
> + local_irq_restore(flags);
> +}

I know you say this is for dual-core chips only, but it would be nice if
you'd write this in a way that doesn't assume that (even if the
corenet-specific timebase freezing comes later).

Do we need an isync after setting the timebase, to ensure it's happened
before we enable the timebase? Likewise, do we need a readback after
disabling the timebase to ensure it's disabled before we read the
timebase in give_timebase?

> atomic_t kexec_down_cpus = ATOMIC_INIT(0);
>
> void mpc85xx_smp_kexec_cpu_down(int crash_shutdown, int secondary)
> @@ -228,6 +286,20 @@ smp_85xx_setup_cpu(int cpu_nr)
> doorbell_setup_this_cpu();
> }
>
> +#ifdef CONFIG_KEXEC
> +static const struct of_device_id guts_ids[] = {
> + { .compatible = "fsl,mpc8572-guts", },
> + { .compatible = "fsl,mpc8560-guts", },
> + { .compatible = "fsl,mpc8536-guts", },
> + { .compatible = "fsl,p1020-guts", },
> + { .compatible = "fsl,p1021-guts", },
> + { .compatible = "fsl,p1022-guts", },
> + { .compatible = "fsl,p1023-guts", },
> + { .compatible = "fsl,p2020-guts", },
> + {},
> +};
> +#endif

MPC8560 and MPC8536 are single-core...

Also please use a more specific name, such as e500v2_smp_guts_ids or
mpc85xx_smp_guts_ids -- when corenet support is added it will likely be
in the same file.

> void __init mpc85xx_smp_init(void)
> {
> struct device_node *np;
> @@ -249,6 +321,19 @@ void __init mpc85xx_smp_init(void)
> smp_85xx_ops.cause_ipi = doorbell_cause_ipi;
> }
>
> +#ifdef CONFIG_KEXEC
> + np = of_find_matching_node(NULL, guts_ids);
> + if (np) {
> + guts = of_iomap(np, 0);
> + smp_85xx_ops.give_timebase = mpc85xx_give_timebase;
> + smp_85xx_ops.take_timebase = mpc85xx_take_timebase;
> + of_node_put(np);
> + } else {
> + smp_85xx_ops.give_timebase = smp_generic_give_timebase;
> + smp_85xx_ops.take_timebase = smp_generic_take_timebase;
> + }

Do not use smp_generic_give/take_timebase, ever. If you don't have the
guts node, then just assume the timebase is already synced.

-Scott

2012-06-01 21:27:38

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 2/5] powerpc/85xx: add HOTPLUG_CPU support

On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
\> +#if defined(CONFIG_FSL_BOOKE) || defined(CONFIG_6xx)
> +extern void __flush_disable_L1(void);
> +#endif

Prototypes aren't normally guarded by ifdefs.

> +static void __cpuinit smp_85xx_mach_cpu_die(void)
> +{
> + unsigned int cpu = smp_processor_id();
> + u32 tmp;
> +
> + local_irq_disable();
> + idle_task_exit();
> + generic_set_cpu_dead(cpu);
> + mb();
> +
> + mtspr(SPRN_TCR, 0);
> +
> + __flush_disable_L1();
> + tmp = (mfspr(SPRN_HID0) & ~(HID0_DOZE|HID0_SLEEP)) | HID0_NAP;
> + mtspr(SPRN_HID0, tmp);
> +
> + /* Enter NAP mode. */
> + tmp = mfmsr();
> + tmp |= MSR_WE;
> + mb();
> + mtmsr(tmp);
> + isync();

Need isync after writing to HID0.

> + /*
> + * We don't set the BPTR register here upon it points
> + * to the boot page properly.
> + */
> + mpic_reset_core(hw_cpu);

That comment's wording is hard to follow -- maybe s/upon it points/since
it already points/

> + /* wait until core is ready... */
> + if (!spin_event_timeout(in_be32(&spin_table->addr_l) == 1,
> + 10000, 100)) {
> + pr_err("%s: timeout waiting for core %d to reset\n",
> + __func__, hw_cpu);
> + ret = -ENOENT;
> + goto out;
> + }

We need to fix U-Boot to write addr_l last (with an msync beforehand).

> -#ifdef CONFIG_KEXEC
> +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU)

Let's not grow lists like this. Is there any harm in building it
unconditionally?

-Scott

2012-06-01 21:54:42

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 3/5] powerpc/85xx: add sleep and deep sleep support

On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> From: Li Yang <[email protected]>
>
> In sleep PM mode, the clocks of e500 core and unused IP blocks is
> turned off. IP blocks which are allowed to wake up the processor
> are still running.
>
> Some Freescale chips like MPC8536 and P1022 has deep sleep PM mode
> in addtion to the sleep PM mode.
>
> While in deep sleep PM mode, additionally, the power supply is
> removed from e500 core and most IP blocks. Only the blocks needed
> to wake up the chip out of deep sleep are ON.
>
> This patch supports 32-bit and 36-bit address space.
>
> The sleep mode is equal to the Standby state in Linux. The deep sleep
> mode is equal to the Suspend-to-RAM state of Linux Power Management.
>
> Command to enter sleep mode.
> echo standby > /sys/power/state
> Command to enter deep sleep mode.
> echo mem > /sys/power/state
>
> Signed-off-by: Dave Liu <[email protected]>
> Signed-off-by: Li Yang <[email protected]>
> Signed-off-by: Jin Qing <[email protected]>
> Signed-off-by: Jerry Huang <[email protected]>
> Cc: Scott Wood <[email protected]>
> Signed-off-by: Zhao Chenhui <[email protected]>
> ---
> Changes for v5:
> * Rename flush_disable_L1 to __flush_disable_L1.
>
> arch/powerpc/Kconfig | 2 +-
> arch/powerpc/include/asm/cacheflush.h | 5 +
> arch/powerpc/kernel/Makefile | 3 +
> arch/powerpc/kernel/l2cache_85xx.S | 53 +++
> arch/powerpc/platforms/85xx/Makefile | 3 +
> arch/powerpc/platforms/85xx/sleep.S | 609 +++++++++++++++++++++++++++++++++
> arch/powerpc/sysdev/fsl_pmc.c | 91 ++++-
> arch/powerpc/sysdev/fsl_soc.h | 5 +
> 8 files changed, 752 insertions(+), 19 deletions(-)
> create mode 100644 arch/powerpc/kernel/l2cache_85xx.S
> create mode 100644 arch/powerpc/platforms/85xx/sleep.S
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d65ae35..039f0a6 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -673,7 +673,7 @@ config FSL_PCI
> config FSL_PMC
> bool
> default y
> - depends on SUSPEND && (PPC_85xx || PPC_86xx)
> + depends on SUSPEND && (PPC_85xx || PPC_86xx) && !PPC_E500MC
> help
> Freescale MPC85xx/MPC86xx power management controller support
> (suspend/resume). For MPC83xx see platforms/83xx/suspend.c
> diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
> index 94ec20a..baa000c 100644
> --- a/arch/powerpc/include/asm/cacheflush.h
> +++ b/arch/powerpc/include/asm/cacheflush.h
> @@ -33,6 +33,11 @@ extern void flush_dcache_page(struct page *page);
> #if defined(CONFIG_FSL_BOOKE) || defined(CONFIG_6xx)
> extern void __flush_disable_L1(void);
> #endif
> +#if defined(CONFIG_FSL_BOOKE)
> +extern void flush_dcache_L1(void);
> +#else
> +#define flush_dcache_L1() do { } while (0)
> +#endif

It doesn't seem right to no-op this on other platforms.

> extern void __flush_icache_range(unsigned long, unsigned long);
> static inline void flush_icache_range(unsigned long start, unsigned long stop)
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index f5808a3..cb70dba 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -64,6 +64,9 @@ obj-$(CONFIG_FA_DUMP) += fadump.o
> ifeq ($(CONFIG_PPC32),y)
> obj-$(CONFIG_E500) += idle_e500.o
> endif
> +ifneq ($(CONFIG_PPC_E500MC),y)
> +obj-$(CONFIG_PPC_85xx) += l2cache_85xx.o
> +endif

Can we introduce a symbol that specifically means pre-e500mc e500,
rather than using negative logic?

I think something like CONFIG_PPC_E500_V1_V2 has been proposed before.

> -static int pmc_probe(struct platform_device *ofdev)
> +static int pmc_probe(struct platform_device *pdev)
> {
> - pmc_regs = of_iomap(ofdev->dev.of_node, 0);
> + struct device_node *np = pdev->dev.of_node;
> +
> + pmc_regs = of_iomap(np, 0);
> if (!pmc_regs)
> return -ENOMEM;
>
> - pmc_dev = &ofdev->dev;
> + pmc_flag = PMC_SLEEP;
> + if (of_device_is_compatible(np, "fsl,mpc8536-pmc"))
> + pmc_flag |= PMC_DEEP_SLEEP;
> +
> + if (of_device_is_compatible(np, "fsl,p1022-pmc"))
> + pmc_flag |= PMC_DEEP_SLEEP;
> +
> suspend_set_ops(&pmc_suspend_ops);
> +
> + pr_info("Freescale PMC driver\n");

If you're going to be noisy on probe, at least provide some useful info
like whether deep sleep or jog are supported.

> return 0;
> }
>
> diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
> index c6d0073..949377d 100644
> --- a/arch/powerpc/sysdev/fsl_soc.h
> +++ b/arch/powerpc/sysdev/fsl_soc.h
> @@ -48,5 +48,10 @@ extern struct platform_diu_data_ops diu_ops;
> void fsl_hv_restart(char *cmd);
> void fsl_hv_halt(void);
>
> +/*
> + * Cast the ccsrbar to 64-bit parameter so that the assembly
> + * code can be compatible with both 32-bit & 36-bit.
> + */
> +extern void mpc85xx_enter_deep_sleep(u64 ccsrbar, u32 powmgtreq);

s/Cast the ccsrbar to 64-bit parameter/ccsrbar is u64 rather than
phys_addr_t/

-Scott

2012-06-01 22:09:00

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source

On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> Add APIs for setting wakeup source and lossless Ethernet in low power modes.
> These APIs can be used by wake-on-packet feature.
>
> Signed-off-by: Dave Liu <[email protected]>
> Signed-off-by: Li Yang <[email protected]>
> Signed-off-by: Jin Qing <[email protected]>
> Signed-off-by: Zhao Chenhui <[email protected]>
> ---
> arch/powerpc/sysdev/fsl_pmc.c | 71 ++++++++++++++++++++++++++++++++++++++++-
> arch/powerpc/sysdev/fsl_soc.h | 9 +++++
> 2 files changed, 79 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/fsl_pmc.c b/arch/powerpc/sysdev/fsl_pmc.c
> index 1dc6e9e..c1170f7 100644
> --- a/arch/powerpc/sysdev/fsl_pmc.c
> +++ b/arch/powerpc/sysdev/fsl_pmc.c
> @@ -34,6 +34,7 @@ struct pmc_regs {
> __be32 powmgtcsr;
> #define POWMGTCSR_SLP 0x00020000
> #define POWMGTCSR_DPSLP 0x00100000
> +#define POWMGTCSR_LOSSLESS 0x00400000
> __be32 res3[2];
> __be32 pmcdr;
> };
> @@ -43,6 +44,74 @@ static unsigned int pmc_flag;
>
> #define PMC_SLEEP 0x1
> #define PMC_DEEP_SLEEP 0x2
> +#define PMC_LOSSLESS 0x4
> +
> +/**
> + * mpc85xx_pmc_set_wake - enable devices as wakeup event source
> + * @pdev: platform device affected
> + * @enable: True to enable event generation; false to disable
> + *
> + * This enables the device as a wakeup event source, or disables it.
> + *
> + * RETURN VALUE:
> + * 0 is returned on success
> + * -EINVAL is returned if device is not supposed to wake up the system
> + * Error code depending on the platform is returned if both the platform and
> + * the native mechanism fail to enable the generation of wake-up events
> + */
> +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool enable)

Why does it have to be a platform_device? Would a bare device_node work
here? If it's for stuff like device_may_wakeup() that could be in a
platform_device wrapper function.

Where does this get called from? I don't see an example user in this
patchset.

> +{
> + int ret = 0;
> + struct device_node *clk_np;
> + u32 *prop;
> + u32 pmcdr_mask;
> +
> + if (!pmc_regs) {
> + pr_err("%s: PMC is unavailable\n", __func__);
> + return -ENODEV;
> + }
> +
> + if (enable && !device_may_wakeup(&pdev->dev))
> + return -EINVAL;

Who is setting can_wakeup for these devices?

> + clk_np = of_parse_phandle(pdev->dev.of_node, "fsl,pmc-handle", 0);
> + if (!clk_np)
> + return -EINVAL;
> +
> + prop = (u32 *)of_get_property(clk_np, "fsl,pmcdr-mask", NULL);

Don't cast the const away.

> + if (!prop) {
> + ret = -EINVAL;
> + goto out;
> + }
> + pmcdr_mask = be32_to_cpup(prop);
> +
> + if (enable)
> + /* clear to enable clock in low power mode */
> + clrbits32(&pmc_regs->pmcdr, pmcdr_mask);
> + else
> + setbits32(&pmc_regs->pmcdr, pmcdr_mask);

What is the default PMCDR if this function is never called? Should init
to all bits set on PM driver probe (or maybe limit it to defined bits
only, though that's a little harder to do generically).

-Scot

2012-06-01 23:31:07

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 5/5] powerpc/85xx: add support to JOG feature using cpufreq interface

On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> Some 85xx silicons like MPC8536 and P1022 have a JOG feature, which provides
> a dynamic mechanism to lower or raise the CPU core clock at runtime.

Is there a reason P1023 isn't supported?

> This patch adds the support to change CPU frequency using the standard
> cpufreq interface. The ratio CORE to CCB can be 1:1(except MPC8536), 3:2,
> 2:1, 5:2, 3:1, 7:2 and 4:1.
>
> Two CPU cores on P1022 must not in the low power state during the frequency
> transition. The driver uses a atomic counter to meet the requirement.
>
> The jog mode frequency transition process on the MPC8536 is similar to
> the deep sleep process. The driver need save the CPU state and restore
> it after CPU warm reset.
>
> Note:
> * The I/O peripherals such as PCIe and eTSEC may lose packets during
> the jog mode frequency transition.

That might be acceptable for eTSEC, but it is not acceptable to lose
anything on PCIe. Especially not if you're going to make this "default y".


> +static int mpc85xx_job_probe(struct platform_device *ofdev)

Job?

> +{
> + struct device_node *np = ofdev->dev.of_node;
> + unsigned int svr;
> +
> + if (of_device_is_compatible(np, "fsl,mpc8536-guts")) {
> + svr = mfspr(SPRN_SVR);
> + if ((svr & 0x7fff) == 0x10) {
> + pr_err("MPC8536 Rev 1.0 do not support JOG.\n");
> + return -ENODEV;
> + }

s/do not support JOG/does not support cpufreq/

> + mpc85xx_freqs = mpc8536_freqs_table;
> + set_pll = mpc8536_set_pll;
> + } else if (of_device_is_compatible(np, "fsl,p1022-guts")) {
> + mpc85xx_freqs = p1022_freqs_table;
> + set_pll = p1022_set_pll;
> + } else {
> + return -ENODEV;
> + }
> +
> + sysfreq = fsl_get_sys_freq();
> +
> + guts = of_iomap(np, 0);
> + if (!guts)
> + return -ENODEV;
> +
> + max_pll[0] = get_pll(0);
> + if (mpc85xx_freqs == p1022_freqs_table)
> + max_pll[1] = get_pll(1);
> +
> + pr_info("Freescale MPC85xx CPU frequency switching(JOG) driver\n");
> +
> + return cpufreq_register_driver(&mpc85xx_cpufreq_driver);
> +}
> +
> +static int mpc85xx_jog_remove(struct platform_device *ofdev)
> +{
> + iounmap(guts);
> + cpufreq_unregister_driver(&mpc85xx_cpufreq_driver);
> +
> + return 0;
> +}
> +
> +static struct of_device_id mpc85xx_jog_ids[] = {
> + { .compatible = "fsl,mpc8536-guts", },
> + { .compatible = "fsl,p1022-guts", },
> + {}
> +};
> +
> +static struct platform_driver mpc85xx_jog_driver = {
> + .driver = {
> + .name = "mpc85xx_cpufreq_jog",
> + .owner = THIS_MODULE,
> + .of_match_table = mpc85xx_jog_ids,
> + },
> + .probe = mpc85xx_job_probe,
> + .remove = mpc85xx_jog_remove,
> +};

Why is this a separate driver from fsl_pmc.c?

Only one driver can bind to a node through normal mechanisms -- you
don't get to take the entire guts node for this.

> +static int __init mpc85xx_jog_init(void)
> +{
> + return platform_driver_register(&mpc85xx_jog_driver);
> +}
> +
> +static void __exit mpc85xx_jog_exit(void)
> +{
> + platform_driver_unregister(&mpc85xx_jog_driver);
> +}
> +
> +module_init(mpc85xx_jog_init);
> +module_exit(mpc85xx_jog_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Dave Liu <[email protected]>");
> diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
> index a35ca44..445bedd 100644
> --- a/arch/powerpc/platforms/Kconfig
> +++ b/arch/powerpc/platforms/Kconfig
> @@ -204,6 +204,17 @@ config CPU_FREQ_PMAC64
> This adds support for frequency switching on Apple iMac G5,
> and some of the more recent desktop G5 machines as well.
>
> +config MPC85xx_CPUFREQ
> + bool "Support for Freescale MPC85xx CPU freq"
> + depends on PPC_85xx && PPC32 && !PPC_E500MC

PPC32 is redundant given the !PPC_E500MC.

> index 8976534..401cac0 100644
> --- a/arch/powerpc/sysdev/fsl_soc.h
> +++ b/arch/powerpc/sysdev/fsl_soc.h
> @@ -62,5 +62,10 @@ void fsl_hv_halt(void);
> * code can be compatible with both 32-bit & 36-bit.
> */
> extern void mpc85xx_enter_deep_sleep(u64 ccsrbar, u32 powmgtreq);
> +
> +static inline void mpc85xx_enter_jog(u64 ccsrbar, u32 powmgtreq)
> +{
> + mpc85xx_enter_deep_sleep(ccsrbar, powmgtreq);
> +}

What value is this function adding over mpc85xx_enter_deep_sleep()?

> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 2060c6e..c4cd342 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -381,6 +381,36 @@ out:
> }
> EXPORT_SYMBOL_GPL(cpu_up);
>
> +/*
> + * Prevent regular CPU hotplug from racing with the freezer, by disabling CPU
> + * hotplug when tasks are about to be frozen. Also, don't allow the freezer
> + * to continue until any currently running CPU hotplug operation gets
> + * completed.
> + * To modify the 'cpu_hotplug_disabled' flag, we need to acquire the
> + * 'cpu_add_remove_lock'. And this same lock is also taken by the regular
> + * CPU hotplug path and released only after it is complete. Thus, we
> + * (and hence the freezer) will block here until any currently running CPU
> + * hotplug operation gets completed.
> + */
> +void cpu_hotplug_disable_before_freeze(void)
> +{
> + cpu_maps_update_begin();
> + cpu_hotplug_disabled = 1;
> + cpu_maps_update_done();
> +}
> +
> +
> +/*
> + * When tasks have been thawed, re-enable regular CPU hotplug (which had been
> + * disabled while beginning to freeze tasks).
> + */
> +void cpu_hotplug_enable_after_thaw(void)
> +{
> + cpu_maps_update_begin();
> + cpu_hotplug_disabled = 0;
> + cpu_maps_update_done();
> +}
> +
> #ifdef CONFIG_PM_SLEEP_SMP
> static cpumask_var_t frozen_cpus;
>
> @@ -479,36 +509,6 @@ static int __init alloc_frozen_cpus(void)
> core_initcall(alloc_frozen_cpus);
>
> /*
> - * Prevent regular CPU hotplug from racing with the freezer, by disabling CPU
> - * hotplug when tasks are about to be frozen. Also, don't allow the freezer
> - * to continue until any currently running CPU hotplug operation gets
> - * completed.
> - * To modify the 'cpu_hotplug_disabled' flag, we need to acquire the
> - * 'cpu_add_remove_lock'. And this same lock is also taken by the regular
> - * CPU hotplug path and released only after it is complete. Thus, we
> - * (and hence the freezer) will block here until any currently running CPU
> - * hotplug operation gets completed.
> - */
> -void cpu_hotplug_disable_before_freeze(void)
> -{
> - cpu_maps_update_begin();
> - cpu_hotplug_disabled = 1;
> - cpu_maps_update_done();
> -}
> -
> -
> -/*
> - * When tasks have been thawed, re-enable regular CPU hotplug (which had been
> - * disabled while beginning to freeze tasks).
> - */
> -void cpu_hotplug_enable_after_thaw(void)
> -{
> - cpu_maps_update_begin();
> - cpu_hotplug_disabled = 0;
> - cpu_maps_update_done();
> -}
> -
> -/*
> * When callbacks for CPU hotplug notifications are being executed, we must
> * ensure that the state of the system with respect to the tasks being frozen
> * or not, as reported by the notification, remains unchanged *throughout the

Why did you need to move this?

-Scott

2012-06-04 11:04:17

by Chenhui Zhao

[permalink] [raw]
Subject: Re: [PATCH v5 2/5] powerpc/85xx: add HOTPLUG_CPU support

On Fri, Jun 01, 2012 at 04:27:27PM -0500, Scott Wood wrote:
> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> \> +#if defined(CONFIG_FSL_BOOKE) || defined(CONFIG_6xx)
> > +extern void __flush_disable_L1(void);
> > +#endif
>
> Prototypes aren't normally guarded by ifdefs.

OK. Thanks.

>
> > +static void __cpuinit smp_85xx_mach_cpu_die(void)
> > +{
> > + unsigned int cpu = smp_processor_id();
> > + u32 tmp;
> > +
> > + local_irq_disable();
> > + idle_task_exit();
> > + generic_set_cpu_dead(cpu);
> > + mb();
> > +
> > + mtspr(SPRN_TCR, 0);
> > +
> > + __flush_disable_L1();
> > + tmp = (mfspr(SPRN_HID0) & ~(HID0_DOZE|HID0_SLEEP)) | HID0_NAP;
> > + mtspr(SPRN_HID0, tmp);
> > +
> > + /* Enter NAP mode. */
> > + tmp = mfmsr();
> > + tmp |= MSR_WE;
> > + mb();
> > + mtmsr(tmp);
> > + isync();
>
> Need isync after writing to HID0.
>
> > + /*
> > + * We don't set the BPTR register here upon it points
> > + * to the boot page properly.
> > + */
> > + mpic_reset_core(hw_cpu);
>
> That comment's wording is hard to follow -- maybe s/upon it points/since
> it already points/
>
> > + /* wait until core is ready... */
> > + if (!spin_event_timeout(in_be32(&spin_table->addr_l) == 1,
> > + 10000, 100)) {
> > + pr_err("%s: timeout waiting for core %d to reset\n",
> > + __func__, hw_cpu);
> > + ret = -ENOENT;
> > + goto out;
> > + }
>
> We need to fix U-Boot to write addr_l last (with an msync beforehand).

I agree.

>
> > -#ifdef CONFIG_KEXEC
> > +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU)
>
> Let's not grow lists like this. Is there any harm in building it
> unconditionally?
>
> -Scott

We need this ifdef. We only set give_timebase/take_timebase
when CONFIG_KEXEC or CONFIG_HOTPLUG_CPU is defined.

-Chenhui

2012-06-04 11:11:31

by Chenhui Zhao

[permalink] [raw]
Subject: Re: [PATCH v5 3/5] powerpc/85xx: add sleep and deep sleep support

On Fri, Jun 01, 2012 at 04:54:35PM -0500, Scott Wood wrote:
> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> > From: Li Yang <[email protected]>
> >
> > In sleep PM mode, the clocks of e500 core and unused IP blocks is
> > turned off. IP blocks which are allowed to wake up the processor
> > are still running.
> >
> > Some Freescale chips like MPC8536 and P1022 has deep sleep PM mode
> > in addtion to the sleep PM mode.
> >
> > While in deep sleep PM mode, additionally, the power supply is
> > removed from e500 core and most IP blocks. Only the blocks needed
> > to wake up the chip out of deep sleep are ON.
> >
> > This patch supports 32-bit and 36-bit address space.
> >
> > The sleep mode is equal to the Standby state in Linux. The deep sleep
> > mode is equal to the Suspend-to-RAM state of Linux Power Management.
> >
> > Command to enter sleep mode.
> > echo standby > /sys/power/state
> > Command to enter deep sleep mode.
> > echo mem > /sys/power/state
> >
> > Signed-off-by: Dave Liu <[email protected]>
> > Signed-off-by: Li Yang <[email protected]>
> > Signed-off-by: Jin Qing <[email protected]>
> > Signed-off-by: Jerry Huang <[email protected]>
> > Cc: Scott Wood <[email protected]>
> > Signed-off-by: Zhao Chenhui <[email protected]>
> > ---
> > Changes for v5:
> > * Rename flush_disable_L1 to __flush_disable_L1.
> >
> > arch/powerpc/Kconfig | 2 +-
> > arch/powerpc/include/asm/cacheflush.h | 5 +
> > arch/powerpc/kernel/Makefile | 3 +
> > arch/powerpc/kernel/l2cache_85xx.S | 53 +++
> > arch/powerpc/platforms/85xx/Makefile | 3 +
> > arch/powerpc/platforms/85xx/sleep.S | 609 +++++++++++++++++++++++++++++++++
> > arch/powerpc/sysdev/fsl_pmc.c | 91 ++++-
> > arch/powerpc/sysdev/fsl_soc.h | 5 +
> > 8 files changed, 752 insertions(+), 19 deletions(-)
> > create mode 100644 arch/powerpc/kernel/l2cache_85xx.S
> > create mode 100644 arch/powerpc/platforms/85xx/sleep.S
> >
> > diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
> > index 94ec20a..baa000c 100644
> > --- a/arch/powerpc/include/asm/cacheflush.h
> > +++ b/arch/powerpc/include/asm/cacheflush.h
> > @@ -33,6 +33,11 @@ extern void flush_dcache_page(struct page *page);
> > #if defined(CONFIG_FSL_BOOKE) || defined(CONFIG_6xx)
> > extern void __flush_disable_L1(void);
> > #endif
> > +#if defined(CONFIG_FSL_BOOKE)
> > +extern void flush_dcache_L1(void);
> > +#else
> > +#define flush_dcache_L1() do { } while (0)
> > +#endif
>
> It doesn't seem right to no-op this on other platforms.

The pmc_suspend_enter() in fsl_pmc.c used by mpc85xx and mpc86xx,
but flush_dcache_L1() have no definition in mpc86xx platform.
I will write flush_dcache_L1() for mpc86xx platform.

>
> > extern void __flush_icache_range(unsigned long, unsigned long);
> > static inline void flush_icache_range(unsigned long start, unsigned long stop)
> > diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> > index f5808a3..cb70dba 100644
> > --- a/arch/powerpc/kernel/Makefile
> > +++ b/arch/powerpc/kernel/Makefile
> > @@ -64,6 +64,9 @@ obj-$(CONFIG_FA_DUMP) += fadump.o
> > ifeq ($(CONFIG_PPC32),y)
> > obj-$(CONFIG_E500) += idle_e500.o
> > endif
> > +ifneq ($(CONFIG_PPC_E500MC),y)
> > +obj-$(CONFIG_PPC_85xx) += l2cache_85xx.o
> > +endif
>
> Can we introduce a symbol that specifically means pre-e500mc e500,
> rather than using negative logic?
>
> I think something like CONFIG_PPC_E500_V1_V2 has been proposed before.

Agree. But CONFIG_PPC_E500_V1_V2 haven't been merged.

>
> > -static int pmc_probe(struct platform_device *ofdev)
> > +static int pmc_probe(struct platform_device *pdev)
> > {
> > - pmc_regs = of_iomap(ofdev->dev.of_node, 0);
> > + struct device_node *np = pdev->dev.of_node;
> > +
> > + pmc_regs = of_iomap(np, 0);
> > if (!pmc_regs)
> > return -ENOMEM;
> >
> > - pmc_dev = &ofdev->dev;
> > + pmc_flag = PMC_SLEEP;
> > + if (of_device_is_compatible(np, "fsl,mpc8536-pmc"))
> > + pmc_flag |= PMC_DEEP_SLEEP;
> > +
> > + if (of_device_is_compatible(np, "fsl,p1022-pmc"))
> > + pmc_flag |= PMC_DEEP_SLEEP;
> > +
> > suspend_set_ops(&pmc_suspend_ops);
> > +
> > + pr_info("Freescale PMC driver\n");
>
> If you're going to be noisy on probe, at least provide some useful info
> like whether deep sleep or jog are supported.
>
> > return 0;
> > }
> >
> > diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
> > index c6d0073..949377d 100644
> > --- a/arch/powerpc/sysdev/fsl_soc.h
> > +++ b/arch/powerpc/sysdev/fsl_soc.h
> > @@ -48,5 +48,10 @@ extern struct platform_diu_data_ops diu_ops;
> > void fsl_hv_restart(char *cmd);
> > void fsl_hv_halt(void);
> >
> > +/*
> > + * Cast the ccsrbar to 64-bit parameter so that the assembly
> > + * code can be compatible with both 32-bit & 36-bit.
> > + */
> > +extern void mpc85xx_enter_deep_sleep(u64 ccsrbar, u32 powmgtreq);
>
> s/Cast the ccsrbar to 64-bit parameter/ccsrbar is u64 rather than
> phys_addr_t/
>
> -Scott

OK. Thanks.

-Chenhui

2012-06-04 11:35:37

by Chenhui Zhao

[permalink] [raw]
Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source

On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:
> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> > Add APIs for setting wakeup source and lossless Ethernet in low power modes.
> > These APIs can be used by wake-on-packet feature.
> >
> > Signed-off-by: Dave Liu <[email protected]>
> > Signed-off-by: Li Yang <[email protected]>
> > Signed-off-by: Jin Qing <[email protected]>
> > Signed-off-by: Zhao Chenhui <[email protected]>
> > ---
> > arch/powerpc/sysdev/fsl_pmc.c | 71 ++++++++++++++++++++++++++++++++++++++++-
> > arch/powerpc/sysdev/fsl_soc.h | 9 +++++
> > 2 files changed, 79 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/powerpc/sysdev/fsl_pmc.c b/arch/powerpc/sysdev/fsl_pmc.c
> > index 1dc6e9e..c1170f7 100644
> > --- a/arch/powerpc/sysdev/fsl_pmc.c
> > +++ b/arch/powerpc/sysdev/fsl_pmc.c
> > @@ -34,6 +34,7 @@ struct pmc_regs {
> > __be32 powmgtcsr;
> > #define POWMGTCSR_SLP 0x00020000
> > #define POWMGTCSR_DPSLP 0x00100000
> > +#define POWMGTCSR_LOSSLESS 0x00400000
> > __be32 res3[2];
> > __be32 pmcdr;
> > };
> > @@ -43,6 +44,74 @@ static unsigned int pmc_flag;
> >
> > #define PMC_SLEEP 0x1
> > #define PMC_DEEP_SLEEP 0x2
> > +#define PMC_LOSSLESS 0x4
> > +
> > +/**
> > + * mpc85xx_pmc_set_wake - enable devices as wakeup event source
> > + * @pdev: platform device affected
> > + * @enable: True to enable event generation; false to disable
> > + *
> > + * This enables the device as a wakeup event source, or disables it.
> > + *
> > + * RETURN VALUE:
> > + * 0 is returned on success
> > + * -EINVAL is returned if device is not supposed to wake up the system
> > + * Error code depending on the platform is returned if both the platform and
> > + * the native mechanism fail to enable the generation of wake-up events
> > + */
> > +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool enable)
>
> Why does it have to be a platform_device? Would a bare device_node work
> here? If it's for stuff like device_may_wakeup() that could be in a
> platform_device wrapper function.

It does not have to be a platform_device. I think it can be a struct device.

>
> Where does this get called from? I don't see an example user in this
> patchset.

It will be used by a gianfar related patch. I plan to submit that patch
after these patches accepted.

>
> > +{
> > + int ret = 0;
> > + struct device_node *clk_np;
> > + u32 *prop;
> > + u32 pmcdr_mask;
> > +
> > + if (!pmc_regs) {
> > + pr_err("%s: PMC is unavailable\n", __func__);
> > + return -ENODEV;
> > + }
> > +
> > + if (enable && !device_may_wakeup(&pdev->dev))
> > + return -EINVAL;
>
> Who is setting can_wakeup for these devices?

The device driver is responsible to set can_wakeup.

>
> > + clk_np = of_parse_phandle(pdev->dev.of_node, "fsl,pmc-handle", 0);
> > + if (!clk_np)
> > + return -EINVAL;
> > +
> > + prop = (u32 *)of_get_property(clk_np, "fsl,pmcdr-mask", NULL);
>
> Don't cast the const away.

OK.

>
> > + if (!prop) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > + pmcdr_mask = be32_to_cpup(prop);
> > +
> > + if (enable)
> > + /* clear to enable clock in low power mode */
> > + clrbits32(&pmc_regs->pmcdr, pmcdr_mask);
> > + else
> > + setbits32(&pmc_regs->pmcdr, pmcdr_mask);
>
> What is the default PMCDR if this function is never called? Should init
> to all bits set on PM driver probe (or maybe limit it to defined bits
> only, though that's a little harder to do generically).
>
> -Scot

The default PMCDR is defined separately by individual chip.
I agree with you. I will have a try.

-Chenhui

2012-06-04 16:32:54

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 2/5] powerpc/85xx: add HOTPLUG_CPU support

On 06/04/2012 06:04 AM, Zhao Chenhui wrote:
> On Fri, Jun 01, 2012 at 04:27:27PM -0500, Scott Wood wrote:
>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>> -#ifdef CONFIG_KEXEC
>>> +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU)
>>
>> Let's not grow lists like this. Is there any harm in building it
>> unconditionally?
>>
>> -Scott
>
> We need this ifdef. We only set give_timebase/take_timebase
> when CONFIG_KEXEC or CONFIG_HOTPLUG_CPU is defined.

If we really need this to be a compile-time decision, make a new symbol
for it, but I really think this should be decided at runtime. Just
because we have kexec or hotplug support enabled doesn't mean that's
actually what we're doing at the moment.

-Scott

2012-06-04 22:58:46

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 3/5] powerpc/85xx: add sleep and deep sleep support

On 06/04/2012 06:12 AM, Zhao Chenhui wrote:
> On Fri, Jun 01, 2012 at 04:54:35PM -0500, Scott Wood wrote:
>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>> diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
>>> index 94ec20a..baa000c 100644
>>> --- a/arch/powerpc/include/asm/cacheflush.h
>>> +++ b/arch/powerpc/include/asm/cacheflush.h
>>> @@ -33,6 +33,11 @@ extern void flush_dcache_page(struct page *page);
>>> #if defined(CONFIG_FSL_BOOKE) || defined(CONFIG_6xx)
>>> extern void __flush_disable_L1(void);
>>> #endif
>>> +#if defined(CONFIG_FSL_BOOKE)
>>> +extern void flush_dcache_L1(void);
>>> +#else
>>> +#define flush_dcache_L1() do { } while (0)
>>> +#endif
>>
>> It doesn't seem right to no-op this on other platforms.
>
> The pmc_suspend_enter() in fsl_pmc.c used by mpc85xx and mpc86xx,
> but flush_dcache_L1() have no definition in mpc86xx platform.
> I will write flush_dcache_L1() for mpc86xx platform.

How about only calling the function when it's needed? If we didn't need
an L1 flush here on 86xx before, why do we need it now?

>>> extern void __flush_icache_range(unsigned long, unsigned long);
>>> static inline void flush_icache_range(unsigned long start, unsigned long stop)
>>> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
>>> index f5808a3..cb70dba 100644
>>> --- a/arch/powerpc/kernel/Makefile
>>> +++ b/arch/powerpc/kernel/Makefile
>>> @@ -64,6 +64,9 @@ obj-$(CONFIG_FA_DUMP) += fadump.o
>>> ifeq ($(CONFIG_PPC32),y)
>>> obj-$(CONFIG_E500) += idle_e500.o
>>> endif
>>> +ifneq ($(CONFIG_PPC_E500MC),y)
>>> +obj-$(CONFIG_PPC_85xx) += l2cache_85xx.o
>>> +endif
>>
>> Can we introduce a symbol that specifically means pre-e500mc e500,
>> rather than using negative logic?
>>
>> I think something like CONFIG_PPC_E500_V1_V2 has been proposed before.
>
> Agree. But CONFIG_PPC_E500_V1_V2 haven't been merged.

Has the concept been NACKed, or just forgotten? If the latter, you
could include it in this patchset.

-Scott

2012-06-04 23:02:42

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source

On 06/04/2012 06:36 AM, Zhao Chenhui wrote:
> On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:
>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>> Add APIs for setting wakeup source and lossless Ethernet in low power modes.
>>> These APIs can be used by wake-on-packet feature.
>>>
>>> Signed-off-by: Dave Liu <[email protected]>
>>> Signed-off-by: Li Yang <[email protected]>
>>> Signed-off-by: Jin Qing <[email protected]>
>>> Signed-off-by: Zhao Chenhui <[email protected]>
>>> ---
>>> arch/powerpc/sysdev/fsl_pmc.c | 71 ++++++++++++++++++++++++++++++++++++++++-
>>> arch/powerpc/sysdev/fsl_soc.h | 9 +++++
>>> 2 files changed, 79 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/powerpc/sysdev/fsl_pmc.c b/arch/powerpc/sysdev/fsl_pmc.c
>>> index 1dc6e9e..c1170f7 100644
>>> --- a/arch/powerpc/sysdev/fsl_pmc.c
>>> +++ b/arch/powerpc/sysdev/fsl_pmc.c
>>> @@ -34,6 +34,7 @@ struct pmc_regs {
>>> __be32 powmgtcsr;
>>> #define POWMGTCSR_SLP 0x00020000
>>> #define POWMGTCSR_DPSLP 0x00100000
>>> +#define POWMGTCSR_LOSSLESS 0x00400000
>>> __be32 res3[2];
>>> __be32 pmcdr;
>>> };
>>> @@ -43,6 +44,74 @@ static unsigned int pmc_flag;
>>>
>>> #define PMC_SLEEP 0x1
>>> #define PMC_DEEP_SLEEP 0x2
>>> +#define PMC_LOSSLESS 0x4
>>> +
>>> +/**
>>> + * mpc85xx_pmc_set_wake - enable devices as wakeup event source
>>> + * @pdev: platform device affected
>>> + * @enable: True to enable event generation; false to disable
>>> + *
>>> + * This enables the device as a wakeup event source, or disables it.
>>> + *
>>> + * RETURN VALUE:
>>> + * 0 is returned on success
>>> + * -EINVAL is returned if device is not supposed to wake up the system
>>> + * Error code depending on the platform is returned if both the platform and
>>> + * the native mechanism fail to enable the generation of wake-up events
>>> + */
>>> +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool enable)
>>
>> Why does it have to be a platform_device? Would a bare device_node work
>> here? If it's for stuff like device_may_wakeup() that could be in a
>> platform_device wrapper function.
>
> It does not have to be a platform_device. I think it can be a struct device.

Why does it even need that? The low level mechanism for influencing
PMCDR should only need a device node, not a Linux device struct.

>> Where does this get called from? I don't see an example user in this
>> patchset.
>
> It will be used by a gianfar related patch. I plan to submit that patch
> after these patches accepted.

It would be nice to see how this is used when reviewing this.

>>> +{
>>> + int ret = 0;
>>> + struct device_node *clk_np;
>>> + u32 *prop;
>>> + u32 pmcdr_mask;
>>> +
>>> + if (!pmc_regs) {
>>> + pr_err("%s: PMC is unavailable\n", __func__);
>>> + return -ENODEV;
>>> + }
>>> +
>>> + if (enable && !device_may_wakeup(&pdev->dev))
>>> + return -EINVAL;
>>
>> Who is setting can_wakeup for these devices?
>
> The device driver is responsible to set can_wakeup.

How would the device driver know how to set it? Wouldn't this depend on
the particular SoC and low power mode?

-Scott

2012-06-05 04:08:31

by Li Yang-R58472

[permalink] [raw]
Subject: RE: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source



> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, June 05, 2012 7:03 AM
> To: Zhao Chenhui-B35336
> Cc: [email protected]; [email protected];
> [email protected]; Li Yang-R58472
> Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup
> event source
>
> On 06/04/2012 06:36 AM, Zhao Chenhui wrote:
> > On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:
> >> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> >>> Add APIs for setting wakeup source and lossless Ethernet in low power
> modes.
> >>> These APIs can be used by wake-on-packet feature.
> >>>
> >>> Signed-off-by: Dave Liu <[email protected]>
> >>> Signed-off-by: Li Yang <[email protected]>
> >>> Signed-off-by: Jin Qing <[email protected]>
> >>> Signed-off-by: Zhao Chenhui <[email protected]>
> >>> ---
> >>> arch/powerpc/sysdev/fsl_pmc.c | 71
> ++++++++++++++++++++++++++++++++++++++++-
> >>> arch/powerpc/sysdev/fsl_soc.h | 9 +++++
> >>> 2 files changed, 79 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/arch/powerpc/sysdev/fsl_pmc.c
> b/arch/powerpc/sysdev/fsl_pmc.c
> >>> index 1dc6e9e..c1170f7 100644
> >>> --- a/arch/powerpc/sysdev/fsl_pmc.c
> >>> +++ b/arch/powerpc/sysdev/fsl_pmc.c
> >>> @@ -34,6 +34,7 @@ struct pmc_regs {
> >>> __be32 powmgtcsr;
> >>> #define POWMGTCSR_SLP 0x00020000
> >>> #define POWMGTCSR_DPSLP 0x00100000
> >>> +#define POWMGTCSR_LOSSLESS 0x00400000
> >>> __be32 res3[2];
> >>> __be32 pmcdr;
> >>> };
> >>> @@ -43,6 +44,74 @@ static unsigned int pmc_flag;
> >>>
> >>> #define PMC_SLEEP 0x1
> >>> #define PMC_DEEP_SLEEP 0x2
> >>> +#define PMC_LOSSLESS 0x4
> >>> +
> >>> +/**
> >>> + * mpc85xx_pmc_set_wake - enable devices as wakeup event source
> >>> + * @pdev: platform device affected
> >>> + * @enable: True to enable event generation; false to disable
> >>> + *
> >>> + * This enables the device as a wakeup event source, or disables it.
> >>> + *
> >>> + * RETURN VALUE:
> >>> + * 0 is returned on success
> >>> + * -EINVAL is returned if device is not supposed to wake up the
> system
> >>> + * Error code depending on the platform is returned if both the
> platform and
> >>> + * the native mechanism fail to enable the generation of wake-up
> events
> >>> + */
> >>> +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool enable)
> >>
> >> Why does it have to be a platform_device? Would a bare device_node
> work
> >> here? If it's for stuff like device_may_wakeup() that could be in a
> >> platform_device wrapper function.
> >
> > It does not have to be a platform_device. I think it can be a struct
> device.
>
> Why does it even need that? The low level mechanism for influencing
> PMCDR should only need a device node, not a Linux device struct.

It does no harm to pass the device structure and makes more sense for object oriented interface design.

>
> >> Where does this get called from? I don't see an example user in this
> >> patchset.
> >
> > It will be used by a gianfar related patch. I plan to submit that patch
> > after these patches accepted.
>
> It would be nice to see how this is used when reviewing this.
>
> >>> +{
> >>> + int ret = 0;
> >>> + struct device_node *clk_np;
> >>> + u32 *prop;
> >>> + u32 pmcdr_mask;
> >>> +
> >>> + if (!pmc_regs) {
> >>> + pr_err("%s: PMC is unavailable\n", __func__);
> >>> + return -ENODEV;
> >>> + }
> >>> +
> >>> + if (enable && !device_may_wakeup(&pdev->dev))
> >>> + return -EINVAL;
> >>
> >> Who is setting can_wakeup for these devices?
> >
> > The device driver is responsible to set can_wakeup.
>
> How would the device driver know how to set it? Wouldn't this depend on
> the particular SoC and low power mode?

It is based on the "fsl,magic-packet" and "fsl,wake-on-filer" device tree properties.

Leo

2012-06-05 09:08:10

by Chenhui Zhao

[permalink] [raw]
Subject: Re: [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync

On Fri, Jun 01, 2012 at 10:40:00AM -0500, Scott Wood wrote:
> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> > #ifdef CONFIG_KEXEC
> > +static struct ccsr_guts __iomem *guts;
> > +static u64 timebase;
> > +static int tb_req;
> > +static int tb_valid;
> > +
> > +static void mpc85xx_timebase_freeze(int freeze)
>
> Why is this under CONFIG_KEXEC? It'll also be needed for CPU hotplug.

Yes, the timebase sync is also needed for CPU hotplug, but this patch is unrelated to CPU hotplug.
I added CONFIG_HOTPLUG_CPU in the next patch.

>
> > +{
> > + unsigned int mask;
> > +
> > + if (!guts)
> > + return;
> > +
> > + mask = CCSR_GUTS_DEVDISR_TB0 | CCSR_GUTS_DEVDISR_TB1;
> > + if (freeze)
> > + setbits32(&guts->devdisr, mask);
> > + else
> > + clrbits32(&guts->devdisr, mask);
> > +
> > + in_be32(&guts->devdisr);
> > +}
> > +
> > +static void mpc85xx_give_timebase(void)
> > +{
> > + unsigned long flags;
> > +
> > + local_irq_save(flags);
> > +
> > + while (!tb_req)
> > + barrier();
> > + tb_req = 0;
> > +
> > + mpc85xx_timebase_freeze(1);
> > + timebase = get_tb();
> > + mb();
> > + tb_valid = 1;
> > +
> > + while (tb_valid)
> > + barrier();
> > +
> > + mpc85xx_timebase_freeze(0);
> > +
> > + local_irq_restore(flags);
> > +}
> > +
> > +static void mpc85xx_take_timebase(void)
> > +{
> > + unsigned long flags;
> > +
> > + local_irq_save(flags);
> > +
> > + tb_req = 1;
> > + while (!tb_valid)
> > + barrier();
> > +
> > + set_tb(timebase >> 32, timebase & 0xffffffff);
> > + mb();
> > + tb_valid = 0;
> > +
> > + local_irq_restore(flags);
> > +}
>
> I know you say this is for dual-core chips only, but it would be nice if
> you'd write this in a way that doesn't assume that (even if the
> corenet-specific timebase freezing comes later).

At this point, I have not thought about how to implement the cornet-specific timebase freezing.

>
> Do we need an isync after setting the timebase, to ensure it's happened
> before we enable the timebase? Likewise, do we need a readback after
> disabling the timebase to ensure it's disabled before we read the
> timebase in give_timebase?

I checked the e500 core manual (Chapter 2.16 Synchronization Requirements for SPRs).
Only some SPR registers need an isync. The timebase registers do not.

I did a readback in mpc85xx_timebase_freeze().

>
> > atomic_t kexec_down_cpus = ATOMIC_INIT(0);
> >
> > void mpc85xx_smp_kexec_cpu_down(int crash_shutdown, int secondary)
> > @@ -228,6 +286,20 @@ smp_85xx_setup_cpu(int cpu_nr)
> > doorbell_setup_this_cpu();
> > }
> >
> > +#ifdef CONFIG_KEXEC
> > +static const struct of_device_id guts_ids[] = {
> > + { .compatible = "fsl,mpc8572-guts", },
> > + { .compatible = "fsl,mpc8560-guts", },
> > + { .compatible = "fsl,mpc8536-guts", },
> > + { .compatible = "fsl,p1020-guts", },
> > + { .compatible = "fsl,p1021-guts", },
> > + { .compatible = "fsl,p1022-guts", },
> > + { .compatible = "fsl,p1023-guts", },
> > + { .compatible = "fsl,p2020-guts", },
> > + {},
> > +};
> > +#endif
>
> MPC8560 and MPC8536 are single-core...

Thanks. I will remove them.

>
> Also please use a more specific name, such as e500v2_smp_guts_ids or
> mpc85xx_smp_guts_ids -- when corenet support is added it will likely be
> in the same file.
>
> > void __init mpc85xx_smp_init(void)
> > {
> > struct device_node *np;
> > @@ -249,6 +321,19 @@ void __init mpc85xx_smp_init(void)
> > smp_85xx_ops.cause_ipi = doorbell_cause_ipi;
> > }
> >
> > +#ifdef CONFIG_KEXEC
> > + np = of_find_matching_node(NULL, guts_ids);
> > + if (np) {
> > + guts = of_iomap(np, 0);
> > + smp_85xx_ops.give_timebase = mpc85xx_give_timebase;
> > + smp_85xx_ops.take_timebase = mpc85xx_take_timebase;
> > + of_node_put(np);
> > + } else {
> > + smp_85xx_ops.give_timebase = smp_generic_give_timebase;
> > + smp_85xx_ops.take_timebase = smp_generic_take_timebase;
> > + }
>
> Do not use smp_generic_give/take_timebase, ever. If you don't have the
> guts node, then just assume the timebase is already synced.
>
> -Scott

smp_generic_give/take_timebase is the default in KEXEC before.
If do not set them, it may make KEXEC fail on other platforms.

-Chenhui

2012-06-05 10:59:09

by Chenhui Zhao

[permalink] [raw]
Subject: Re: [PATCH v5 5/5] powerpc/85xx: add support to JOG feature using cpufreq interface

On Fri, Jun 01, 2012 at 06:30:55PM -0500, Scott Wood wrote:
> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> > Some 85xx silicons like MPC8536 and P1022 have a JOG feature, which provides
> > a dynamic mechanism to lower or raise the CPU core clock at runtime.
>
> Is there a reason P1023 isn't supported?

P1023 support is deferred.

>
> > This patch adds the support to change CPU frequency using the standard
> > cpufreq interface. The ratio CORE to CCB can be 1:1(except MPC8536), 3:2,
> > 2:1, 5:2, 3:1, 7:2 and 4:1.
> >
> > Two CPU cores on P1022 must not in the low power state during the frequency
> > transition. The driver uses a atomic counter to meet the requirement.
> >
> > The jog mode frequency transition process on the MPC8536 is similar to
> > the deep sleep process. The driver need save the CPU state and restore
> > it after CPU warm reset.
> >
> > Note:
> > * The I/O peripherals such as PCIe and eTSEC may lose packets during
> > the jog mode frequency transition.
>
> That might be acceptable for eTSEC, but it is not acceptable to lose
> anything on PCIe. Especially not if you're going to make this "default y".

It is a hardware limitation. Peripherals in the platform will not be operating
during the jog mode frequency transition process.

I can make it "default n".

>
>
> > +static int mpc85xx_job_probe(struct platform_device *ofdev)
>
> Job?

Sorry, It should be "jog".

>
> > +{
> > + struct device_node *np = ofdev->dev.of_node;
> > + unsigned int svr;
> > +
> > + if (of_device_is_compatible(np, "fsl,mpc8536-guts")) {
> > + svr = mfspr(SPRN_SVR);
> > + if ((svr & 0x7fff) == 0x10) {
> > + pr_err("MPC8536 Rev 1.0 do not support JOG.\n");
> > + return -ENODEV;
> > + }
>
> s/do not support JOG/does not support cpufreq/
>
> > + mpc85xx_freqs = mpc8536_freqs_table;
> > + set_pll = mpc8536_set_pll;
> > + } else if (of_device_is_compatible(np, "fsl,p1022-guts")) {
> > + mpc85xx_freqs = p1022_freqs_table;
> > + set_pll = p1022_set_pll;
> > + } else {
> > + return -ENODEV;
> > + }
> > +
> > + sysfreq = fsl_get_sys_freq();
> > +
> > + guts = of_iomap(np, 0);
> > + if (!guts)
> > + return -ENODEV;
> > +
> > + max_pll[0] = get_pll(0);
> > + if (mpc85xx_freqs == p1022_freqs_table)
> > + max_pll[1] = get_pll(1);
> > +
> > + pr_info("Freescale MPC85xx CPU frequency switching(JOG) driver\n");
> > +
> > + return cpufreq_register_driver(&mpc85xx_cpufreq_driver);
> > +}
> > +
> > +static int mpc85xx_jog_remove(struct platform_device *ofdev)
> > +{
> > + iounmap(guts);
> > + cpufreq_unregister_driver(&mpc85xx_cpufreq_driver);
> > +
> > + return 0;
> > +}
> > +
> > +static struct of_device_id mpc85xx_jog_ids[] = {
> > + { .compatible = "fsl,mpc8536-guts", },
> > + { .compatible = "fsl,p1022-guts", },
> > + {}
> > +};
> > +
> > +static struct platform_driver mpc85xx_jog_driver = {
> > + .driver = {
> > + .name = "mpc85xx_cpufreq_jog",
> > + .owner = THIS_MODULE,
> > + .of_match_table = mpc85xx_jog_ids,
> > + },
> > + .probe = mpc85xx_job_probe,
> > + .remove = mpc85xx_jog_remove,
> > +};
>
> Why is this a separate driver from fsl_pmc.c?
>
> Only one driver can bind to a node through normal mechanisms -- you
> don't get to take the entire guts node for this.

You are right. I will not bind this to the guts node.

>
> > +static int __init mpc85xx_jog_init(void)
> > +{
> > + return platform_driver_register(&mpc85xx_jog_driver);
> > +}
> > +
> > +static void __exit mpc85xx_jog_exit(void)
> > +{
> > + platform_driver_unregister(&mpc85xx_jog_driver);
> > +}
> > +
> > +module_init(mpc85xx_jog_init);
> > +module_exit(mpc85xx_jog_exit);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Dave Liu <[email protected]>");
> > diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
> > index a35ca44..445bedd 100644
> > --- a/arch/powerpc/platforms/Kconfig
> > +++ b/arch/powerpc/platforms/Kconfig
> > @@ -204,6 +204,17 @@ config CPU_FREQ_PMAC64
> > This adds support for frequency switching on Apple iMac G5,
> > and some of the more recent desktop G5 machines as well.
> >
> > +config MPC85xx_CPUFREQ
> > + bool "Support for Freescale MPC85xx CPU freq"
> > + depends on PPC_85xx && PPC32 && !PPC_E500MC
>
> PPC32 is redundant given the !PPC_E500MC.

Yes.

>
> > index 8976534..401cac0 100644
> > --- a/arch/powerpc/sysdev/fsl_soc.h
> > +++ b/arch/powerpc/sysdev/fsl_soc.h
> > @@ -62,5 +62,10 @@ void fsl_hv_halt(void);
> > * code can be compatible with both 32-bit & 36-bit.
> > */
> > extern void mpc85xx_enter_deep_sleep(u64 ccsrbar, u32 powmgtreq);
> > +
> > +static inline void mpc85xx_enter_jog(u64 ccsrbar, u32 powmgtreq)
> > +{
> > + mpc85xx_enter_deep_sleep(ccsrbar, powmgtreq);
> > +}
>
> What value is this function adding over mpc85xx_enter_deep_sleep()?

Just an alias name. If this is improper, I could use mpc85xx_enter_deep_sleep() directly.

-Chenhui

2012-06-05 11:17:55

by Chenhui Zhao

[permalink] [raw]
Subject: Re: [PATCH v5 2/5] powerpc/85xx: add HOTPLUG_CPU support

On Mon, Jun 04, 2012 at 11:32:47AM -0500, Scott Wood wrote:
> On 06/04/2012 06:04 AM, Zhao Chenhui wrote:
> > On Fri, Jun 01, 2012 at 04:27:27PM -0500, Scott Wood wrote:
> >> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> >>> -#ifdef CONFIG_KEXEC
> >>> +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU)
> >>
> >> Let's not grow lists like this. Is there any harm in building it
> >> unconditionally?
> >>
> >> -Scott
> >
> > We need this ifdef. We only set give_timebase/take_timebase
> > when CONFIG_KEXEC or CONFIG_HOTPLUG_CPU is defined.
>
> If we really need this to be a compile-time decision, make a new symbol
> for it, but I really think this should be decided at runtime. Just
> because we have kexec or hotplug support enabled doesn't mean that's
> actually what we're doing at the moment.
>
> -Scott

If user does not enable kexec or hotplug, these codes are redundant.
So use CONFIG_KEXEC and CONFIG_HOTPLUG_CPU to gard them.

-Chenhui

2012-06-05 11:35:00

by Chenhui Zhao

[permalink] [raw]
Subject: Re: [PATCH v5 3/5] powerpc/85xx: add sleep and deep sleep support

On Mon, Jun 04, 2012 at 05:58:38PM -0500, Scott Wood wrote:
> On 06/04/2012 06:12 AM, Zhao Chenhui wrote:
> > On Fri, Jun 01, 2012 at 04:54:35PM -0500, Scott Wood wrote:
> >> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> >>> diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
> >>> index 94ec20a..baa000c 100644
> >>> --- a/arch/powerpc/include/asm/cacheflush.h
> >>> +++ b/arch/powerpc/include/asm/cacheflush.h
> >>> @@ -33,6 +33,11 @@ extern void flush_dcache_page(struct page *page);
> >>> #if defined(CONFIG_FSL_BOOKE) || defined(CONFIG_6xx)
> >>> extern void __flush_disable_L1(void);
> >>> #endif
> >>> +#if defined(CONFIG_FSL_BOOKE)
> >>> +extern void flush_dcache_L1(void);
> >>> +#else
> >>> +#define flush_dcache_L1() do { } while (0)
> >>> +#endif
> >>
> >> It doesn't seem right to no-op this on other platforms.
> >
> > The pmc_suspend_enter() in fsl_pmc.c used by mpc85xx and mpc86xx,
> > but flush_dcache_L1() have no definition in mpc86xx platform.
> > I will write flush_dcache_L1() for mpc86xx platform.
>
> How about only calling the function when it's needed? If we didn't need
> an L1 flush here on 86xx before, why do we need it now?

How about using CONFIG_PPC_85xx to gard it, like this.

case PM_SUSPEND_STANDBY:
local_irq_disable();
#ifdef CONFIG_PPC_85xx
flush_dcache_L1();
#endif
setbits32(&pmc_regs->powmgtcsr, POWMGTCSR_SLP);

>
> >>> extern void __flush_icache_range(unsigned long, unsigned long);
> >>> static inline void flush_icache_range(unsigned long start, unsigned long stop)
> >>> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> >>> index f5808a3..cb70dba 100644
> >>> --- a/arch/powerpc/kernel/Makefile
> >>> +++ b/arch/powerpc/kernel/Makefile
> >>> @@ -64,6 +64,9 @@ obj-$(CONFIG_FA_DUMP) += fadump.o
> >>> ifeq ($(CONFIG_PPC32),y)
> >>> obj-$(CONFIG_E500) += idle_e500.o
> >>> endif
> >>> +ifneq ($(CONFIG_PPC_E500MC),y)
> >>> +obj-$(CONFIG_PPC_85xx) += l2cache_85xx.o
> >>> +endif
> >>
> >> Can we introduce a symbol that specifically means pre-e500mc e500,
> >> rather than using negative logic?
> >>
> >> I think something like CONFIG_PPC_E500_V1_V2 has been proposed before.
> >
> > Agree. But CONFIG_PPC_E500_V1_V2 haven't been merged.
>
> Has the concept been NACKed, or just forgotten? If the latter, you
> could include it in this patchset.
>
> -Scott

In patchwork, it's state is "Superseded".
http://patchwork.ozlabs.org/patch/124284/

-Chenhui

2012-06-05 15:58:50

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 5/5] powerpc/85xx: add support to JOG feature using cpufreq interface

On 06/05/2012 05:59 AM, Zhao Chenhui wrote:
> On Fri, Jun 01, 2012 at 06:30:55PM -0500, Scott Wood wrote:
>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>> The jog mode frequency transition process on the MPC8536 is similar to
>>> the deep sleep process. The driver need save the CPU state and restore
>>> it after CPU warm reset.
>>>
>>> Note:
>>> * The I/O peripherals such as PCIe and eTSEC may lose packets during
>>> the jog mode frequency transition.
>>
>> That might be acceptable for eTSEC, but it is not acceptable to lose
>> anything on PCIe. Especially not if you're going to make this "default y".
>
> It is a hardware limitation.

Then make sure jog isn't used if PCIe is used.

Maybe you could do something with the suspend infrastructure, but this
is sufficiently heavyweight that transitions should be manually
requested, not triggered by the automatic cpufreq governor.

Does this apply to p1022, or just mpc8536?

> Peripherals in the platform will not be operating
> during the jog mode frequency transition process.

What ensures this?

-Scott

2012-06-05 16:07:51

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync

On 06/05/2012 04:08 AM, Zhao Chenhui wrote:
> On Fri, Jun 01, 2012 at 10:40:00AM -0500, Scott Wood wrote:
>> I know you say this is for dual-core chips only, but it would be nice if
>> you'd write this in a way that doesn't assume that (even if the
>> corenet-specific timebase freezing comes later).
>
> At this point, I have not thought about how to implement the cornet-specific timebase freezing.

I wasn't asking you to. I was asking you to not have logic that breaks
with more than 2 CPUs.

>> Do we need an isync after setting the timebase, to ensure it's happened
>> before we enable the timebase? Likewise, do we need a readback after
>> disabling the timebase to ensure it's disabled before we read the
>> timebase in give_timebase?
>
> I checked the e500 core manual (Chapter 2.16 Synchronization Requirements for SPRs).
> Only some SPR registers need an isync. The timebase registers do not.

I don't trust that, and the consequences of having the sync be imperfect
are too unpleasant to chance it.

> I did a readback in mpc85xx_timebase_freeze().

Sorry, missed that somehow.

>>> +#ifdef CONFIG_KEXEC
>>> + np = of_find_matching_node(NULL, guts_ids);
>>> + if (np) {
>>> + guts = of_iomap(np, 0);
>>> + smp_85xx_ops.give_timebase = mpc85xx_give_timebase;
>>> + smp_85xx_ops.take_timebase = mpc85xx_take_timebase;
>>> + of_node_put(np);
>>> + } else {
>>> + smp_85xx_ops.give_timebase = smp_generic_give_timebase;
>>> + smp_85xx_ops.take_timebase = smp_generic_take_timebase;
>>> + }
>>
>> Do not use smp_generic_give/take_timebase, ever. If you don't have the
>> guts node, then just assume the timebase is already synced.
>>
>> -Scott
>
> smp_generic_give/take_timebase is the default in KEXEC before.

That was a mistake.

> If do not set them, it may make KEXEC fail on other platforms.

What platforms?

-Scott

2012-06-05 16:11:46

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source

On 06/04/2012 11:08 PM, Li Yang-R58472 wrote:
>
>
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Tuesday, June 05, 2012 7:03 AM
>> To: Zhao Chenhui-B35336
>> Cc: [email protected]; [email protected];
>> [email protected]; Li Yang-R58472
>> Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup
>> event source
>>
>> On 06/04/2012 06:36 AM, Zhao Chenhui wrote:
>>> On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:
>>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>>>> +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool enable)
>>>>
>>>> Why does it have to be a platform_device? Would a bare device_node
>> work
>>>> here? If it's for stuff like device_may_wakeup() that could be in a
>>>> platform_device wrapper function.
>>>
>>> It does not have to be a platform_device. I think it can be a struct
>> device.
>>
>> Why does it even need that? The low level mechanism for influencing
>> PMCDR should only need a device node, not a Linux device struct.
>
> It does no harm to pass the device structure and makes more sense for object oriented interface design.

It does do harm if you don't have a device structure to pass, if for
some reason you found the device by directly looking for it rather than
going through the device model.

>>>> Who is setting can_wakeup for these devices?
>>>
>>> The device driver is responsible to set can_wakeup.
>>
>> How would the device driver know how to set it? Wouldn't this depend on
>> the particular SoC and low power mode?
>
> It is based on the "fsl,magic-packet" and "fsl,wake-on-filer" device tree properties.

fsl,magic-packet was a mistake. It is equivalent to checking the
compatible for etsec. It does not convey any information about whether
the eTSEC is still active in a given low power mode.

How is fsl,wake-os-filer relevant to this decision? When will it be set
but not fsl,magic-packet?

What about devices other than ethernet? What about differences between
ordinary sleep and deep sleep?

-Scott

2012-06-05 16:13:59

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 3/5] powerpc/85xx: add sleep and deep sleep support

On 06/05/2012 06:35 AM, Zhao Chenhui wrote:
> On Mon, Jun 04, 2012 at 05:58:38PM -0500, Scott Wood wrote:
>> On 06/04/2012 06:12 AM, Zhao Chenhui wrote:
>>> On Fri, Jun 01, 2012 at 04:54:35PM -0500, Scott Wood wrote:
>>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>>>> diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
>>>>> index 94ec20a..baa000c 100644
>>>>> --- a/arch/powerpc/include/asm/cacheflush.h
>>>>> +++ b/arch/powerpc/include/asm/cacheflush.h
>>>>> @@ -33,6 +33,11 @@ extern void flush_dcache_page(struct page *page);
>>>>> #if defined(CONFIG_FSL_BOOKE) || defined(CONFIG_6xx)
>>>>> extern void __flush_disable_L1(void);
>>>>> #endif
>>>>> +#if defined(CONFIG_FSL_BOOKE)
>>>>> +extern void flush_dcache_L1(void);
>>>>> +#else
>>>>> +#define flush_dcache_L1() do { } while (0)
>>>>> +#endif
>>>>
>>>> It doesn't seem right to no-op this on other platforms.
>>>
>>> The pmc_suspend_enter() in fsl_pmc.c used by mpc85xx and mpc86xx,
>>> but flush_dcache_L1() have no definition in mpc86xx platform.
>>> I will write flush_dcache_L1() for mpc86xx platform.
>>
>> How about only calling the function when it's needed? If we didn't need
>> an L1 flush here on 86xx before, why do we need it now?
>
> How about using CONFIG_PPC_85xx to gard it, like this.
>
> case PM_SUSPEND_STANDBY:
> local_irq_disable();
> #ifdef CONFIG_PPC_85xx
> flush_dcache_L1();
> #endif
> setbits32(&pmc_regs->powmgtcsr, POWMGTCSR_SLP);

We don't support building 85xx/86xx in the same kernel and likely never
will, so this is OK.

>>>> Can we introduce a symbol that specifically means pre-e500mc e500,
>>>> rather than using negative logic?
>>>>
>>>> I think something like CONFIG_PPC_E500_V1_V2 has been proposed before.
>>>
>>> Agree. But CONFIG_PPC_E500_V1_V2 haven't been merged.
>>
>> Has the concept been NACKed, or just forgotten? If the latter, you
>> could include it in this patchset.
>>
>> -Scott
>
> In patchwork, it's state is "Superseded".
> http://patchwork.ozlabs.org/patch/124284/

I still think there's value in adding such a symbol.

-Scott

2012-06-05 16:16:08

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 2/5] powerpc/85xx: add HOTPLUG_CPU support

On 06/05/2012 06:18 AM, Zhao Chenhui wrote:
> On Mon, Jun 04, 2012 at 11:32:47AM -0500, Scott Wood wrote:
>> On 06/04/2012 06:04 AM, Zhao Chenhui wrote:
>>> On Fri, Jun 01, 2012 at 04:27:27PM -0500, Scott Wood wrote:
>>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>>>> -#ifdef CONFIG_KEXEC
>>>>> +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU)
>>>>
>>>> Let's not grow lists like this. Is there any harm in building it
>>>> unconditionally?
>>>>
>>>> -Scott
>>>
>>> We need this ifdef. We only set give_timebase/take_timebase
>>> when CONFIG_KEXEC or CONFIG_HOTPLUG_CPU is defined.
>>
>> If we really need this to be a compile-time decision, make a new symbol
>> for it, but I really think this should be decided at runtime. Just
>> because we have kexec or hotplug support enabled doesn't mean that's
>> actually what we're doing at the moment.
>>
>> -Scott
>
> If user does not enable kexec or hotplug, these codes are redundant.
> So use CONFIG_KEXEC and CONFIG_HOTPLUG_CPU to gard them.

My point is that these lists tend to grow and be a maintenance pain.
For small things it's often better to not worry about saving a few
bytes. For larger things that need to be conditional, define a new
symbol rather than growing ORed lists like this.

-Scott

2012-06-05 16:49:43

by Li Yang-R58472

[permalink] [raw]
Subject: RE: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source



> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, June 06, 2012 12:12 AM
> To: Li Yang-R58472
> Cc: Wood Scott-B07421; Zhao Chenhui-B35336; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup
> event source
>
> On 06/04/2012 11:08 PM, Li Yang-R58472 wrote:
> >
> >
> >> -----Original Message-----
> >> From: Wood Scott-B07421
> >> Sent: Tuesday, June 05, 2012 7:03 AM
> >> To: Zhao Chenhui-B35336
> >> Cc: [email protected]; [email protected];
> >> [email protected]; Li Yang-R58472
> >> Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as
> >> wakeup event source
> >>
> >> On 06/04/2012 06:36 AM, Zhao Chenhui wrote:
> >>> On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:
> >>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> >>>>> +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool
> >>>>> +enable)
> >>>>
> >>>> Why does it have to be a platform_device? Would a bare device_node
> >> work
> >>>> here? If it's for stuff like device_may_wakeup() that could be in
> >>>> a platform_device wrapper function.
> >>>
> >>> It does not have to be a platform_device. I think it can be a struct
> >> device.
> >>
> >> Why does it even need that? The low level mechanism for influencing
> >> PMCDR should only need a device node, not a Linux device struct.
> >
> > It does no harm to pass the device structure and makes more sense for
> object oriented interface design.
>
> It does do harm if you don't have a device structure to pass, if for some
> reason you found the device by directly looking for it rather than going
> through the device model.

Whether or not a device is a wakeup source not only depends on the SoC specification but also the configuration and current state for the device. I only expect the device driver to have this knowledge and call this function rather than some standalone platform code. Therefore I don't think your concern matters.

>
> >>>> Who is setting can_wakeup for these devices?
> >>>
> >>> The device driver is responsible to set can_wakeup.
> >>
> >> How would the device driver know how to set it? Wouldn't this depend
> >> on the particular SoC and low power mode?
> >
> > It is based on the "fsl,magic-packet" and "fsl,wake-on-filer" device
> tree properties.
>
> fsl,magic-packet was a mistake. It is equivalent to checking the
> compatible for etsec. It does not convey any information about whether

It can be described either by explicit feature property or by the compatible. I don't think it is a problem that we choose one against another.

> the eTSEC is still active in a given low power mode.

Whether or not the eTSEC is still active in both sleep and deep sleep is only depending on if we set it to be a wakeup source. If it behaves differently for low power modes in the future, we could address that by adding additional property.

>
> How is fsl,wake-os-filer relevant to this decision? When will it be set
> but not fsl,magic-packet?

I mean either fsl,magic-packet or fsl,wake-on-filer shows it can be a wakeup source. Currently we don't have an SoC to have wake-on-filer while not wake-on-magic. But I think it's better to consider both as they are independent features.

>
> What about devices other than ethernet? What about differences between
> ordinary sleep and deep sleep?

There is no difference for sleep and deep sleep for all wakeup sources currently. We can address the problem if it is not the case in the future.

Leo

2012-06-05 18:05:24

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source

On 06/05/2012 11:49 AM, Li Yang-R58472 wrote:
>
>
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Wednesday, June 06, 2012 12:12 AM
>> To: Li Yang-R58472
>> Cc: Wood Scott-B07421; Zhao Chenhui-B35336; [email protected];
>> [email protected]; [email protected]
>> Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup
>> event source
>>
>> On 06/04/2012 11:08 PM, Li Yang-R58472 wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Wood Scott-B07421
>>>> Sent: Tuesday, June 05, 2012 7:03 AM
>>>> To: Zhao Chenhui-B35336
>>>> Cc: [email protected]; [email protected];
>>>> [email protected]; Li Yang-R58472
>>>> Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as
>>>> wakeup event source
>>>>
>>>> On 06/04/2012 06:36 AM, Zhao Chenhui wrote:
>>>>> On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:
>>>>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>>>>>> +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool
>>>>>>> +enable)
>>>>>>
>>>>>> Why does it have to be a platform_device? Would a bare device_node
>>>> work
>>>>>> here? If it's for stuff like device_may_wakeup() that could be in
>>>>>> a platform_device wrapper function.
>>>>>
>>>>> It does not have to be a platform_device. I think it can be a struct
>>>> device.
>>>>
>>>> Why does it even need that? The low level mechanism for influencing
>>>> PMCDR should only need a device node, not a Linux device struct.
>>>
>>> It does no harm to pass the device structure and makes more sense for
>> object oriented interface design.
>>
>> It does do harm if you don't have a device structure to pass, if for some
>> reason you found the device by directly looking for it rather than going
>> through the device model.
>
> Whether or not a device is a wakeup source not only depends on the
> SoC specification but also the configuration and current state for
> the device. I only expect the device driver to have this knowledge
> and call this function rather than some standalone platform code.
> Therefore I don't think your concern matters.

First, I think it's bad API to force the passing of a higher level
object when a lower level object would suffice (and there are no
legitimate future-proofing or abstraction reasons for hiding the lower
level object).

But regardless, the entity you call a "device driver" may or may not use
the standard driver model (e.g. look at PCI root complexes), and your
assumption about what platform code knows may or may not be correct. I
could just as well say that only platform code knows about the SoC
clock/power routing during a given low power state.

>>>>>> Who is setting can_wakeup for these devices?
>>>>>
>>>>> The device driver is responsible to set can_wakeup.
>>>>
>>>> How would the device driver know how to set it? Wouldn't this depend
>>>> on the particular SoC and low power mode?
>>>
>>> It is based on the "fsl,magic-packet" and "fsl,wake-on-filer" device
>> tree properties.
>>
>> fsl,magic-packet was a mistake. It is equivalent to checking the
>> compatible for etsec. It does not convey any information about whether
>
> It can be described either by explicit feature property or by the
> compatible. I don't think it is a problem that we choose one against
> another.

I do think it's a problem, because it's unnecessarily complicated, and
more error prone (we probably didn't have fsl,magic-packet on all the
SoCs that support it before the .dtsi refactoring). But my point was
that it says nothing about whether the eTSEC will still be functioning
during a low power state.

>> the eTSEC is still active in a given low power mode.
>
> Whether or not the eTSEC is still active in both sleep and deep sleep
> is only depending on if we set it to be a wakeup source.

Only because we happen to support eTSEC as a wakeup source on all SoCs.

> If it behaves differently for low power modes in the future, we could
> address that by adding additional property.
>
>>
>> How is fsl,wake-os-filer relevant to this decision? When will it be set
>> but not fsl,magic-packet?
>
> I mean either fsl,magic-packet or fsl,wake-on-filer shows it can be a
> wakeup source. Currently we don't have an SoC to have wake-on-filer
> while not wake-on-magic. But I think it's better to consider both as
> they are independent features.

You're not willing to consider an SoC where waking on eTSEC is
unsupported, but you're willing to consider an eTSEC that has
wake-on-filer but magic packet support has for some reason been dropped?

>> What about devices other than ethernet? What about differences between
>> ordinary sleep and deep sleep?
>
> There is no difference for sleep and deep sleep for all wakeup sources currently.

I recall being able to wake an mpc85xx chip (maybe mpc8544?) from
ordinary sleep using the DUART (even though the manual suggests that
only external interrupts can be used -- not even eTSEC). You can't do
that in deep sleep.

You ignored "what about devices other than ethernet".

-Scott

2012-06-06 04:06:25

by Li Yang

[permalink] [raw]
Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source

On Wed, Jun 6, 2012 at 2:05 AM, Scott Wood <[email protected]> wrote:
> On 06/05/2012 11:49 AM, Li Yang-R58472 wrote:
>>
>>

>>>>> On 06/04/2012 06:36 AM, Zhao Chenhui wrote:
>>>>>> On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:
>>>>>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>>>>>>> +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool
>>>>>>>> +enable)
>>>>>>>
>>>>>>> Why does it have to be a platform_device?  Would a bare device_node
>>>>> work
>>>>>>> here?  If it's for stuff like device_may_wakeup() that could be in
>>>>>>> a platform_device wrapper function.
>>>>>>
>>>>>> It does not have to be a platform_device. I think it can be a struct
>>>>> device.
>>>>>
>>>>> Why does it even need that?  The low level mechanism for influencing
>>>>> PMCDR should only need a device node, not a Linux device struct.
>>>>
>>>> It does no harm to pass the device structure and makes more sense for
>>> object oriented interface design.
>>>
>>> It does do harm if you don't have a device structure to pass, if for some
>>> reason you found the device by directly looking for it rather than going
>>> through the device model.
>>
>> Whether or not a device is a wakeup source not only depends on the
>> SoC specification but also the configuration and current state for
>> the device.  I only expect the device driver to have this knowledge
>> and call this function rather than some standalone platform code.
>> Therefore I don't think your concern matters.
>
> First, I think it's bad API to force the passing of a higher level
> object when a lower level object would suffice (and there are no
> legitimate future-proofing or abstraction reasons for hiding the lower
> level object).
>
> But regardless, the entity you call a "device driver" may or may not use
> the standard driver model (e.g. look at PCI root complexes), and your
> assumption about what platform code knows may or may not be correct.  I
> could just as well say that only platform code knows about the SoC
> clock/power routing during a given low power state.

Good point. We need to fix such non-standard drivers. The new PM
framework depends a lot on the standard Linux Driver Model. We need
to change our drivers to make them work better with PM. Also we have
already submitted a patch series to change the PCI root complex driver
in that regard.

>
>>>>>>> Who is setting can_wakeup for these devices?
>>>>>>
>>>>>> The device driver is responsible to set can_wakeup.
>>>>>
>>>>> How would the device driver know how to set it?  Wouldn't this depend
>>>>> on the particular SoC and low power mode?
>>>>
>>>> It is based on the "fsl,magic-packet" and "fsl,wake-on-filer" device
>>> tree properties.
>>>
>>> fsl,magic-packet was a mistake.  It is equivalent to checking the
>>> compatible for etsec.  It does not convey any information about whether
>>
>> It can be described either by explicit feature property or by the
>> compatible.  I don't think it is a problem that we choose one against
>> another.
>
> I do think it's a problem, because it's unnecessarily complicated, and
> more error prone (we probably didn't have fsl,magic-packet on all the
> SoCs that support it before the .dtsi refactoring).  But my point was
> that it says nothing about whether the eTSEC will still be functioning
> during a low power state.
>
>>> the eTSEC is still active in a given low power mode.
>>
>> Whether or not the eTSEC is still active in both sleep and deep sleep
>> is only depending on if we set it to be a wakeup source.
>
> Only because we happen to support eTSEC as a wakeup source on all SoCs.
>
>> If it behaves differently for low power modes in the future, we could
>> address that by adding additional property.
>>
>>>
>>> How is fsl,wake-os-filer relevant to this decision?  When will it be set
>>> but not fsl,magic-packet?
>>
>> I mean either fsl,magic-packet or fsl,wake-on-filer shows it can be a
>> wakeup source.  Currently we don't have an SoC to have wake-on-filer
>> while not wake-on-magic.  But I think it's better to consider both as
>> they are independent features.
>
> You're not willing to consider an SoC where waking on eTSEC is
> unsupported, but you're willing to consider an eTSEC that has
> wake-on-filer but magic packet support has for some reason been dropped?

Good findings. :) I think it's fine to keep the extra that have
already been done as long as it does no harm. But we should stop
adding more extras that are not necessary for now.

>
>>> What about devices other than ethernet?  What about differences between
>>> ordinary sleep and deep sleep?
>>
>> There is no difference for sleep and deep sleep for all wakeup sources currently.
>
> I recall being able to wake an mpc85xx chip (maybe mpc8544?) from
> ordinary sleep using the DUART (even though the manual suggests that
> only external interrupts can be used -- not even eTSEC).  You can't do
> that in deep sleep.

I doubt that as the blocks are clock gated in sleep. We only have
MPC8536 and P1022 in the e500 family to support deep sleep now. I
agree with you the sleep and deep sleep can imply different wakeup
source in the future. But can we worry about it later?

>
> You ignored "what about devices other than ethernet".

No, I haven't. Other devices are so at least for now.

- Leo

2012-06-06 09:31:11

by Chenhui Zhao

[permalink] [raw]
Subject: Re: [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync

On Tue, Jun 05, 2012 at 11:07:41AM -0500, Scott Wood wrote:
> On 06/05/2012 04:08 AM, Zhao Chenhui wrote:
> > On Fri, Jun 01, 2012 at 10:40:00AM -0500, Scott Wood wrote:
> >> I know you say this is for dual-core chips only, but it would be nice if
> >> you'd write this in a way that doesn't assume that (even if the
> >> corenet-specific timebase freezing comes later).
> >
> > At this point, I have not thought about how to implement the cornet-specific timebase freezing.
>
> I wasn't asking you to. I was asking you to not have logic that breaks
> with more than 2 CPUs.

These routines only called in the dual-core case.

>
> >> Do we need an isync after setting the timebase, to ensure it's happened
> >> before we enable the timebase? Likewise, do we need a readback after
> >> disabling the timebase to ensure it's disabled before we read the
> >> timebase in give_timebase?
> >
> > I checked the e500 core manual (Chapter 2.16 Synchronization Requirements for SPRs).
> > Only some SPR registers need an isync. The timebase registers do not.
>
> I don't trust that, and the consequences of having the sync be imperfect
> are too unpleasant to chance it.
>
> > I did a readback in mpc85xx_timebase_freeze().
>
> Sorry, missed that somehow.
>
> >>> +#ifdef CONFIG_KEXEC
> >>> + np = of_find_matching_node(NULL, guts_ids);
> >>> + if (np) {
> >>> + guts = of_iomap(np, 0);
> >>> + smp_85xx_ops.give_timebase = mpc85xx_give_timebase;
> >>> + smp_85xx_ops.take_timebase = mpc85xx_take_timebase;
> >>> + of_node_put(np);
> >>> + } else {
> >>> + smp_85xx_ops.give_timebase = smp_generic_give_timebase;
> >>> + smp_85xx_ops.take_timebase = smp_generic_take_timebase;
> >>> + }
> >>
> >> Do not use smp_generic_give/take_timebase, ever. If you don't have the
> >> guts node, then just assume the timebase is already synced.
> >>
> >> -Scott
> >
> > smp_generic_give/take_timebase is the default in KEXEC before.
>
> That was a mistake.
>
> > If do not set them, it may make KEXEC fail on other platforms.
>
> What platforms?
>
> -Scott

Such as P4080, P3041, etc.

-Chenhui

2012-06-06 09:59:39

by Chenhui Zhao

[permalink] [raw]
Subject: Re: [PATCH v5 2/5] powerpc/85xx: add HOTPLUG_CPU support

On Tue, Jun 05, 2012 at 11:15:52AM -0500, Scott Wood wrote:
> On 06/05/2012 06:18 AM, Zhao Chenhui wrote:
> > On Mon, Jun 04, 2012 at 11:32:47AM -0500, Scott Wood wrote:
> >> On 06/04/2012 06:04 AM, Zhao Chenhui wrote:
> >>> On Fri, Jun 01, 2012 at 04:27:27PM -0500, Scott Wood wrote:
> >>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> >>>>> -#ifdef CONFIG_KEXEC
> >>>>> +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU)
> >>>>
> >>>> Let's not grow lists like this. Is there any harm in building it
> >>>> unconditionally?
> >>>>
> >>>> -Scott
> >>>
> >>> We need this ifdef. We only set give_timebase/take_timebase
> >>> when CONFIG_KEXEC or CONFIG_HOTPLUG_CPU is defined.
> >>
> >> If we really need this to be a compile-time decision, make a new symbol
> >> for it, but I really think this should be decided at runtime. Just
> >> because we have kexec or hotplug support enabled doesn't mean that's
> >> actually what we're doing at the moment.
> >>
> >> -Scott
> >
> > If user does not enable kexec or hotplug, these codes are redundant.
> > So use CONFIG_KEXEC and CONFIG_HOTPLUG_CPU to gard them.
>
> My point is that these lists tend to grow and be a maintenance pain.
> For small things it's often better to not worry about saving a few
> bytes. For larger things that need to be conditional, define a new
> symbol rather than growing ORed lists like this.
>
> -Scott

I agree with you in principle. But there are only two config options
in this patch, and it is unlikely to grow.

-Chenhui

2012-06-06 10:18:27

by Chenhui Zhao

[permalink] [raw]
Subject: Re: [PATCH v5 5/5] powerpc/85xx: add support to JOG feature using cpufreq interface

On Tue, Jun 05, 2012 at 10:58:41AM -0500, Scott Wood wrote:
> On 06/05/2012 05:59 AM, Zhao Chenhui wrote:
> > On Fri, Jun 01, 2012 at 06:30:55PM -0500, Scott Wood wrote:
> >> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> >>> The jog mode frequency transition process on the MPC8536 is similar to
> >>> the deep sleep process. The driver need save the CPU state and restore
> >>> it after CPU warm reset.
> >>>
> >>> Note:
> >>> * The I/O peripherals such as PCIe and eTSEC may lose packets during
> >>> the jog mode frequency transition.
> >>
> >> That might be acceptable for eTSEC, but it is not acceptable to lose
> >> anything on PCIe. Especially not if you're going to make this "default y".
> >
> > It is a hardware limitation.
>
> Then make sure jog isn't used if PCIe is used.
>
> Maybe you could do something with the suspend infrastructure, but this
> is sufficiently heavyweight that transitions should be manually
> requested, not triggered by the automatic cpufreq governor.
>
> Does this apply to p1022, or just mpc8536?

Both of them.

>
> > Peripherals in the platform will not be operating
> > during the jog mode frequency transition process.
>
> What ensures this?
>
> -Scott

Hardware ensures it without software intervention.

-Chenhui

2012-06-06 18:20:17

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 2/5] powerpc/85xx: add HOTPLUG_CPU support

On 06/06/2012 04:59 AM, Zhao Chenhui wrote:
> On Tue, Jun 05, 2012 at 11:15:52AM -0500, Scott Wood wrote:
>> On 06/05/2012 06:18 AM, Zhao Chenhui wrote:
>>> If user does not enable kexec or hotplug, these codes are redundant.
>>> So use CONFIG_KEXEC and CONFIG_HOTPLUG_CPU to gard them.
>>
>> My point is that these lists tend to grow and be a maintenance pain.
>> For small things it's often better to not worry about saving a few
>> bytes. For larger things that need to be conditional, define a new
>> symbol rather than growing ORed lists like this.
>>
>> -Scott
>
> I agree with you in principle. But there are only two config options
> in this patch, and it is unlikely to grow.

That's what everybody says when these things start. :-)

-Scott

2012-06-06 18:28:56

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync

On 06/06/2012 04:31 AM, Zhao Chenhui wrote:
> On Tue, Jun 05, 2012 at 11:07:41AM -0500, Scott Wood wrote:
>> On 06/05/2012 04:08 AM, Zhao Chenhui wrote:
>>> On Fri, Jun 01, 2012 at 10:40:00AM -0500, Scott Wood wrote:
>>>> I know you say this is for dual-core chips only, but it would be nice if
>>>> you'd write this in a way that doesn't assume that (even if the
>>>> corenet-specific timebase freezing comes later).
>>>
>>> At this point, I have not thought about how to implement the cornet-specific timebase freezing.
>>
>> I wasn't asking you to. I was asking you to not have logic that breaks
>> with more than 2 CPUs.
>
> These routines only called in the dual-core case.

Come on, you know we have chips with more than two cores. Why design
such a limitation into it, just because you're not personally interested
in supporting anything but e500v2?

Is it so hard to make it work for an arbitrary number of cores?

>>> If do not set them, it may make KEXEC fail on other platforms.
>>
>> What platforms?
>
> Such as P4080, P3041, etc.

So we need to wait for corenet timebase sync before we stop causing
problems in virtualization, simulators, etc. if a kernel has kexec or
cpu hotplug enabled (whether used or not)?

Can you at least make sure we're actually in a kexec/hotplug scenario at
runtime?

Or just implement corenet timebase sync -- it's not that different.

-Scott

2012-06-06 18:30:42

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source

On 06/05/2012 11:06 PM, Li Yang wrote:
> On Wed, Jun 6, 2012 at 2:05 AM, Scott Wood <[email protected]> wrote:
>> You ignored "what about devices other than ethernet".
>
> No, I haven't. Other devices are so at least for now.

I don't understand that last sentence. Other devices are what?

-Scott

2012-06-07 04:06:37

by Chenhui Zhao

[permalink] [raw]
Subject: Re: [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync

On Wed, Jun 06, 2012 at 01:26:16PM -0500, Scott Wood wrote:
> On 06/06/2012 04:31 AM, Zhao Chenhui wrote:
> > On Tue, Jun 05, 2012 at 11:07:41AM -0500, Scott Wood wrote:
> >> On 06/05/2012 04:08 AM, Zhao Chenhui wrote:
> >>> On Fri, Jun 01, 2012 at 10:40:00AM -0500, Scott Wood wrote:
> >>>> I know you say this is for dual-core chips only, but it would be nice if
> >>>> you'd write this in a way that doesn't assume that (even if the
> >>>> corenet-specific timebase freezing comes later).
> >>>
> >>> At this point, I have not thought about how to implement the cornet-specific timebase freezing.
> >>
> >> I wasn't asking you to. I was asking you to not have logic that breaks
> >> with more than 2 CPUs.
> >
> > These routines only called in the dual-core case.
>
> Come on, you know we have chips with more than two cores. Why design
> such a limitation into it, just because you're not personally interested
> in supporting anything but e500v2?
>
> Is it so hard to make it work for an arbitrary number of cores?
>
> >>> If do not set them, it may make KEXEC fail on other platforms.
> >>
> >> What platforms?
> >
> > Such as P4080, P3041, etc.
>
> So we need to wait for corenet timebase sync before we stop causing
> problems in virtualization, simulators, etc. if a kernel has kexec or
> cpu hotplug enabled (whether used or not)?
>
> Can you at least make sure we're actually in a kexec/hotplug scenario at
> runtime?
>
> Or just implement corenet timebase sync -- it's not that different.
>
> -Scott

We also work on the corenet timebase sync. Our plan is first the dual-core case,
then the case of more than 2 cores. We will submit the corenet timebase sync patch soon.

-Chenhui

2012-06-07 04:10:30

by Li Yang

[permalink] [raw]
Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source

On Thu, Jun 7, 2012 at 2:29 AM, Scott Wood <[email protected]> wrote:
> On 06/05/2012 11:06 PM, Li Yang wrote:
>> On Wed, Jun 6, 2012 at 2:05 AM, Scott Wood <[email protected]> wrote:
>>> You ignored "what about devices other than ethernet".
>>
>> No, I haven't.  Other devices are so at least for now.
>
> I don't understand that last sentence.  Other devices are what?

Probably I misunderstood your question "what about devices other than
ethernet". Did you mean how would other devices other than ethernet
know how to set it?

Other wakeup capable devices can call the API when it is up and
running. It will be the pmc driver's responsibility to find out if
that specific device can be configured as a wakeup source for the SoC.

Leo