Re: [PATCH v4 4/4] ARM: EXYNOS: add Exynos Dual Cluster Support
On Tue, 26 Nov 2013, Vyacheslav Tyrtov wrote:
> From: Tarek Dakhran
>
> Add EDCS(Exynos Dual Cluster Support) for Samsung Exynos5410 SoC.
> This enables all 8 cores, 4 x A7 and 4 x A15 run at the same time.
>
> Signed-off-by: Tarek Dakhran
> Signed-off-by: Vyacheslav Tyrtov
Just some minor comments in addition to those Dave already provided.
[...]
> +/*
> + * Enable cluster-level coherency, in preparation for turning on the MMU.
> + */
> +static void __naked edcs_power_up_setup(unsigned int affinity_level)
> +{
> + asm volatile ("\n"
> + "b cci_enable_port_for_self");
> +}
You should need to turn on the CCI port only for the first CPU to power
up a cluster. This is indicated by the affinity level passed into r0.
See tc2_pm_power_up_setup for example.
> +static int __init edcs_init(void)
> +{
> + int ret;
> + struct device_node *node;
> +
> + node = of_find_compatible_node(NULL, NULL, "samsung,exynos5410");
> + if (!node)
> + return -ENODEV;
> +
> + if (!cci_probed())
> + return -ENODEV;
> +
> + /*
> + * Future entries into the kernel can now go
> + * through the cluster entry vectors.
> + */
> + writel_relaxed(virt_to_phys(mcpm_entry_point), REG_ENTRY_ADDR);
> +
> + edcs_data_init();
> + mcpm_smp_set_ops();
> +
> + ret = mcpm_platform_register(&edcs_power_ops);
> + if (!ret) {
> + mcpm_sync_init(edcs_power_up_setup);
> + pr_info("EDCS power management initialized\n");
> + }
> + return ret;
> +}
Here I'd suggest initializing EG_ENTRY_ADDR only after mcpm_sync_init()
is done. If ever a secondary CPU, seeing that the address is no longer
zero, decides to enter the kernel while the state machine is not
initialized yet, might lead to all sorts of funny behaviors.
Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 4/4] ARM: EXYNOS: add Exynos Dual Cluster Support
On Tue, Nov 26, 2013 at 12:58:08PM +0400, Vyacheslav Tyrtov wrote:
> From: Tarek Dakhran
>
> Add EDCS(Exynos Dual Cluster Support) for Samsung Exynos5410 SoC.
> This enables all 8 cores, 4 x A7 and 4 x A15 run at the same time.
>
> Signed-off-by: Tarek Dakhran
> Signed-off-by: Vyacheslav Tyrtov
> ---
> arch/arm/mach-exynos/Makefile | 2 +
> arch/arm/mach-exynos/edcs.c | 297
> ++
> 2 files changed, 299 insertions(+)
> create mode 100644 arch/arm/mach-exynos/edcs.c
>
> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> index 8930b66..bc1f7f9 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -34,3 +34,5 @@ AFLAGS_exynos-smc.o :=-Wa,-march=armv7-a$(plus_sec)
>
> obj-$(CONFIG_ARCH_EXYNOS4) += mach-exynos4-dt.o
> obj-$(CONFIG_ARCH_EXYNOS5) += mach-exynos5-dt.o
> +
> +obj-$(CONFIG_SOC_EXYNOS5410) += edcs.o
> diff --git a/arch/arm/mach-exynos/edcs.c b/arch/arm/mach-exynos/edcs.c
> new file mode 100644
> index 000..29f0bdd
> --- /dev/null
> +++ b/arch/arm/mach-exynos/edcs.c
> @@ -0,0 +1,297 @@
> +/*
> + * arch/arm/mach-exynos/edcs.c - exynos dual cluster power management support
> + *
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> + * Author: Tarek Dakhran
> + *
> + * 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.
> + *
> + * EDCS(exynos dual cluster support) for Exynos5410 SoC.
> + */
> +
> +#define pr_fmt(fmt) "%s: " fmt, __func__
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#define EDCS_CPUS_PER_CLUSTER4
> +#define EDCS_CLUSTERS2
> +
> +/* Exynos5410 power management registers */
> +#define EDCS_CORE_CONFIGURATION(_nr) (S5P_ARM_CORE0_CONFIGURATION\
> + + ((_nr) * 0x80))
> +#define EDCS_CORE_STATUS(_nr)(EDCS_CORE_CONFIGURATION(_nr) +
> 0x4)
> +#define EDCS_CORE_OPTION(_nr)(EDCS_CORE_CONFIGURATION(_nr) +
> 0x8)
> +
> +#define REG_CPU_STATE_ADDR0 (S5P_VA_SYSRAM_NS + 0x28)
> +#define REG_CPU_STATE_ADDR(_nr) (REG_CPU_STATE_ADDR0 + \
> + (_nr) * EDCS_CPUS_PER_CLUSTER)
> +
> +#define SECONDARY_RESET (1 << 1)
> +#define REG_ENTRY_ADDR (S5P_VA_SYSRAM_NS + 0x1c)
> +
> +#define EDCS_CORE_PWR_ON 0x3
> +#define EDCS_CORE_PWR_OFF0x0
> +#define CORE_PWR_STATE_MASK 0x3
> +
> +static arch_spinlock_t edcs_lock = __ARCH_SPIN_LOCK_UNLOCKED;
> +
> +static int edcs_use_count[EDCS_CPUS_PER_CLUSTER][EDCS_CLUSTERS];
> +static int core_count[EDCS_CLUSTERS];
> +
> +/*
> + * this_core_to_pcpu reads mpidr and defines cluster and cpu.
> + */
> +static void this_core_to_pcpu(unsigned int *pcpu, unsigned int *pcluster)
> +{
> + unsigned int mpidr;
> +
> + mpidr = read_cpuid_mpidr();
> + *pcpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> + *pcluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> +}
> +
> +/*
> + * core_power_state is used to get core power state.
> + * returns:
> + *0x0 - powered off;
> + *0x3 - powered on;
> + *other values - in process;
> + */
> +static int core_power_state(unsigned int cpu, unsigned int cluster)
> +{
> + unsigned int offset = cluster * EDCS_CPUS_PER_CLUSTER + cpu;
> + int status = readl_relaxed(EDCS_CORE_STATUS(offset));
> +
> + return status & CORE_PWR_STATE_MASK;
> +}
> +
> +static void edcs_core_power_up(unsigned int cpu, unsigned int cluster)
> +{
> + unsigned int offset = cluster * EDCS_CPUS_PER_CLUSTER + cpu;
> + if (core_power_state(cpu, cluster) == EDCS_CORE_PWR_OFF) {
This still looks racy.
edcs_core_power_up and edcs_core_power_down are both called with
edcs_lock held, and cluster/cpu ref counting is done before we
get here. So we know that the power state *change* is really supposed
to happen.
*If* my assumption is correct that there is a delay between requesting a
new power state, and the change being reported as completed in
EDCS_CORE_STATUS:
For edcs_core_power_up(), there may be a pending power-down
request which is not visible in the EDCS_CORE_STATUS registers yet.
I think this may mean that a power-up request will simply be missed
if the core status does not read as EDCS_CORE_PWR_OFF yet. If that
can happen, then you need to wait -- calling the power_down_finish
method may be the right thing to do.
However, we cannot sleep here because edcs_lock is held and IRQs
are masked. It may make sense to factor the status check out of
power_down_finish and make it a separate function, then make
edcs_core_power_up() return an error code to edcs_power_u
[PATCH v4 4/4] ARM: EXYNOS: add Exynos Dual Cluster Support
From: Tarek Dakhran
Add EDCS(Exynos Dual Cluster Support) for Samsung Exynos5410 SoC.
This enables all 8 cores, 4 x A7 and 4 x A15 run at the same time.
Signed-off-by: Tarek Dakhran
Signed-off-by: Vyacheslav Tyrtov
---
arch/arm/mach-exynos/Makefile | 2 +
arch/arm/mach-exynos/edcs.c | 297 ++
2 files changed, 299 insertions(+)
create mode 100644 arch/arm/mach-exynos/edcs.c
diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index 8930b66..bc1f7f9 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -34,3 +34,5 @@ AFLAGS_exynos-smc.o :=-Wa,-march=armv7-a$(plus_sec)
obj-$(CONFIG_ARCH_EXYNOS4) += mach-exynos4-dt.o
obj-$(CONFIG_ARCH_EXYNOS5) += mach-exynos5-dt.o
+
+obj-$(CONFIG_SOC_EXYNOS5410) += edcs.o
diff --git a/arch/arm/mach-exynos/edcs.c b/arch/arm/mach-exynos/edcs.c
new file mode 100644
index 000..29f0bdd
--- /dev/null
+++ b/arch/arm/mach-exynos/edcs.c
@@ -0,0 +1,297 @@
+/*
+ * arch/arm/mach-exynos/edcs.c - exynos dual cluster power management support
+ *
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ * Author: Tarek Dakhran
+ *
+ * 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.
+ *
+ * EDCS(exynos dual cluster support) for Exynos5410 SoC.
+ */
+
+#define pr_fmt(fmt)"%s: " fmt, __func__
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#define EDCS_CPUS_PER_CLUSTER 4
+#define EDCS_CLUSTERS 2
+
+/* Exynos5410 power management registers */
+#define EDCS_CORE_CONFIGURATION(_nr) (S5P_ARM_CORE0_CONFIGURATION\
+ + ((_nr) * 0x80))
+#define EDCS_CORE_STATUS(_nr) (EDCS_CORE_CONFIGURATION(_nr) + 0x4)
+#define EDCS_CORE_OPTION(_nr) (EDCS_CORE_CONFIGURATION(_nr) + 0x8)
+
+#define REG_CPU_STATE_ADDR0(S5P_VA_SYSRAM_NS + 0x28)
+#define REG_CPU_STATE_ADDR(_nr)(REG_CPU_STATE_ADDR0 + \
+(_nr) * EDCS_CPUS_PER_CLUSTER)
+
+#define SECONDARY_RESET(1 << 1)
+#define REG_ENTRY_ADDR (S5P_VA_SYSRAM_NS + 0x1c)
+
+#define EDCS_CORE_PWR_ON 0x3
+#define EDCS_CORE_PWR_OFF 0x0
+#define CORE_PWR_STATE_MASK0x3
+
+static arch_spinlock_t edcs_lock = __ARCH_SPIN_LOCK_UNLOCKED;
+
+static int edcs_use_count[EDCS_CPUS_PER_CLUSTER][EDCS_CLUSTERS];
+static int core_count[EDCS_CLUSTERS];
+
+/*
+ * this_core_to_pcpu reads mpidr and defines cluster and cpu.
+ */
+static void this_core_to_pcpu(unsigned int *pcpu, unsigned int *pcluster)
+{
+ unsigned int mpidr;
+
+ mpidr = read_cpuid_mpidr();
+ *pcpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+ *pcluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+}
+
+/*
+ * core_power_state is used to get core power state.
+ * returns:
+ *0x0 - powered off;
+ *0x3 - powered on;
+ *other values - in process;
+ */
+static int core_power_state(unsigned int cpu, unsigned int cluster)
+{
+ unsigned int offset = cluster * EDCS_CPUS_PER_CLUSTER + cpu;
+ int status = readl_relaxed(EDCS_CORE_STATUS(offset));
+
+ return status & CORE_PWR_STATE_MASK;
+}
+
+static void edcs_core_power_up(unsigned int cpu, unsigned int cluster)
+{
+ unsigned int offset = cluster * EDCS_CPUS_PER_CLUSTER + cpu;
+ if (core_power_state(cpu, cluster) == EDCS_CORE_PWR_OFF) {
+ /* boot flag should be written before powering up */
+ wmb();
+ writel_relaxed(EDCS_CORE_PWR_ON,
+EDCS_CORE_CONFIGURATION(offset));
+ }
+}
+
+static void edcs_core_power_down(unsigned int cpu, unsigned int cluster)
+{
+ unsigned int offset = cluster * EDCS_CPUS_PER_CLUSTER + cpu;
+ if (core_power_state(cpu, cluster) == EDCS_CORE_PWR_ON)
+ writel_relaxed(EDCS_CORE_PWR_OFF,
+EDCS_CORE_CONFIGURATION(offset));
+}
+
+void set_boot_flag(unsigned int cpu, unsigned int mode)
+{
+ writel_relaxed(mode, REG_CPU_STATE_ADDR(cpu));
+}
+
+static int edcs_power_up(unsigned int cpu, unsigned int cluster)
+{
+ pr_debug("cpu %u cluster %u\n", cpu, cluster);
+ BUG_ON(cpu >= EDCS_CPUS_PER_CLUSTER || cluster >= EDCS_CLUSTERS);
+
+ local_irq_disable();
+ arch_spin_lock(&edcs_lock);
+
+ edcs_use_count[cpu][cluster]++;
+ if (edcs_use_count[cpu][cluster] == 1) {
+ ++core_count[cluster];
+ set_boot_flag(cpu, SECONDARY_RESET);
+ edcs_core_power_up(cpu, cluster);
+ } else if (edcs_use_count[cpu][cluster] != 2) {
+ /*
+* The only possible values are:
+
