Re: [PATCH RFT 1/2] drivers: bus: check cci device tree node status
On Wed, 10 Dec 2014, Abhilash Kesavan wrote: Hi, On Fri, Nov 28, 2014 at 8:20 PM, Abhilash Kesavan a.kesa...@samsung.com wrote: The arm-cci driver completes the probe sequence even if the cci node is marked as disabled. Add a check in the driver to honour the cci status in the device tree. Signed-off-by: Abhilash Kesavan a.kesa...@samsung.com This patch helps disable CCI on the Arndale Octa board thus resolving some imprecise aborts seen on that board. Kindly review. Acked-by: Nicolas Pitre n...@linaro.org Regards, Abhilash --- drivers/bus/arm-cci.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c index 860da40..0ce5e2d 100644 --- a/drivers/bus/arm-cci.c +++ b/drivers/bus/arm-cci.c @@ -1312,6 +1312,9 @@ static int cci_probe(void) if (!np) return -ENODEV; + if (!of_device_is_available(np)) + return -ENODEV; + cci_config = of_match_node(arm_cci_matches, np)-data; if (!cci_config) return -ENODEV; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: exynos_defconfig: disable CONFIG_EXYNOS5420_MCPM; not stable
On Thu, 27 Nov 2014, Abhilash Kesavan wrote:
Hi Kevin,
On Thu, Nov 27, 2014 at 12:11 AM, Nicolas Pitre
nicolas.pi...@linaro.org wrote:
On Wed, 26 Nov 2014, Kevin Hilman wrote:
Abhilash Kesavan kesavan.abhil...@gmail.com writes:
Hi Kevin,
On Wed, Nov 26, 2014 at 6:30 AM, Kevin Hilman khil...@kernel.org wrote:
[...]
More specifically, with only the loopback call to turn off CCI commented
out, the imprecise aborts go away.
I can't see how enabling snoops for the boot cluster is causing these
aborts. Perhaps as Krzysztof commented it has something to do with the
secure firmware/tz software on these boards ? Other than there does
not appear to be any difference between the working/non-working
setups.
Perhaps the secure firmware is preventing the CCI to be enabled by the
kernel, and that is causing the imprecise abort?
That is well possible.
Now.. if the bootloader/firmware does not let Linux deal with both
the CCI and caches then MCPM simply has no more purpose for this board.
The whole point of MCPM is actually to handle the CCI properly and the
most efficient way despite all the possible races and opportunities for
memory corruptions. And yes, this is a complex task.
So there is actually two choices: the firmware let Linux take care of it
via the MCPM layer (easy), or the firmware has to implement it all
_properly_ (hard) behind an interface such as PSCI, at which point MCPM
should be configured out.
If the firmware does not let Linux interact with the CCI _and_ does not
implement full MCPM-like services then the platform is broken and only a
firmware upgrade could fix that. It might still be possible to boot all
CPUs through other means, but power management would then be severely
limited.
How about restricting the mcpm initialization to only known working
boards like chromebooks and smdk. This would be better than disabling
the config altogether from exynos_defconfig. The non-working boards
would then default to platsmp. Assuming that the firmware handles all
CCI/cache activities then platsmp may work for secondary core boot-up
?
Can you please apply the below diff and test the non-working boards
with CONFIG_EXYNOS5420_MCPM enabled.
I'd much prefer if the CCI is non accessible on some board that the
device tree file for that board be modified instead by marking the CCI
as unavailable.
diff --git a/arch/arm/mach-exynos/mcpm-exynos.c
b/arch/arm/mach-exynos/mcpm-exynos.c
index b0d3c2e..34d77bb 100644
--- a/arch/arm/mach-exynos/mcpm-exynos.c
+++ b/arch/arm/mach-exynos/mcpm-exynos.c
@@ -316,8 +316,9 @@ static void __init exynos_cache_off(void)
}
static const struct of_device_id exynos_dt_mcpm_match[] = {
- { .compatible = samsung,exynos5420 },
- { .compatible = samsung,exynos5800 },
+ { .compatible = samsung,smdk5420 },
+ { .compatible = google,pi },
+ { .compatible = google,pit },
{},
};
On a different note, I did not see any mainline support for Odroid
Xu3, are you testing this board with a non-mainline kernel ?
Regards,
Abhilash
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: exynos_defconfig: disable CONFIG_EXYNOS5420_MCPM; not stable
On Wed, 26 Nov 2014, Kevin Hilman wrote: Abhilash Kesavan kesavan.abhil...@gmail.com writes: Hi Kevin, On Wed, Nov 26, 2014 at 6:30 AM, Kevin Hilman khil...@kernel.org wrote: [...] More specifically, with only the loopback call to turn off CCI commented out, the imprecise aborts go away. I can't see how enabling snoops for the boot cluster is causing these aborts. Perhaps as Krzysztof commented it has something to do with the secure firmware/tz software on these boards ? Other than there does not appear to be any difference between the working/non-working setups. Perhaps the secure firmware is preventing the CCI to be enabled by the kernel, and that is causing the imprecise abort? That is well possible. Now.. if the bootloader/firmware does not let Linux deal with both the CCI and caches then MCPM simply has no more purpose for this board. The whole point of MCPM is actually to handle the CCI properly and the most efficient way despite all the possible races and opportunities for memory corruptions. And yes, this is a complex task. So there is actually two choices: the firmware let Linux take care of it via the MCPM layer (easy), or the firmware has to implement it all _properly_ (hard) behind an interface such as PSCI, at which point MCPM should be configured out. If the firmware does not let Linux interact with the CCI _and_ does not implement full MCPM-like services then the platform is broken and only a firmware upgrade could fix that. It might still be possible to boot all CPUs through other means, but power management would then be severely limited. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: EXYNOS: Fix MCPM build with SUSPEND=n
On Mon, 14 Jul 2014, Krzysztof Kozlowski wrote: Building of EXYNOS5420_MCPM with disabled SUSPEND fails: arch/arm/mach-exynos/built-in.o: In function `exynos_mcpm_init': arch/arm/mach-exynos/mcpm-exynos.c:361: undefined reference to `mcpm_loopback' The exynos_mcpm_init() in mcp-exynos.c calls mcpm_loopback() which depends on cpu_suspend function (ARM_CPU_SUSPEND). Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com Cc: sta...@vger.kernel.org No need to CC stable here as this code has not reached a released kernel yet. Fixes: a6a4d3152e3cbb6 (ARM: 8083/1: exynos: activate the CCI on boot CPU/cluster using the MCPM loopback) And this is not clear yet if this commit ID is stable. Other than that: Acked-by: Nicolas Pitre n...@linaro.org Please send to RMK's patch system. --- arch/arm/mach-exynos/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig index 8f9b66c4ac78..5d4ff6571dcd 100644 --- a/arch/arm/mach-exynos/Kconfig +++ b/arch/arm/mach-exynos/Kconfig @@ -119,6 +119,7 @@ config EXYNOS5420_MCPM bool Exynos5420 Multi-Cluster PM support depends on MCPM SOC_EXYNOS5420 select ARM_CCI + select ARM_CPU_SUSPEND help This is needed to provide CPU and cluster power management on Exynos5420 implementing big.LITTLE. -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cpuidle: big.LITTLE: add MCPM dependency
On Mon, 14 Jul 2014, Arnd Bergmann wrote: 662322fcb6d (cpuidle: big.LITTLE: Add ARCH_EXYNOS entry in config) made it possible for the big-little cpuidle driver to run on exynos, which may or may not include MCPM support at compile time, so we run into a link error when it is disabled: drivers/built-in.o: In function `bl_enter_powerdown': :(.text+0x1889a0): undefined reference to `mcpm_cpu_powered_up' drivers/built-in.o: In function `bl_powerdown_finisher': :(.text+0x1889e8): undefined reference to `mcpm_set_entry_vector' :(.text+0x1889ec): undefined reference to `mcpm_cpu_suspend' This adds an explicit dependency to CONFIG_MCPM to avoid that case. Signed-off-by: Arnd Bergmann a...@arndb.de Cc: Chander Kashyap chander.kash...@linaro.org Cc: Tomasz Figa t.f...@samsung.com Cc: Daniel Lezcano daniel.lezc...@linaro.org --- I believe the broken commit is only present in the samsung/for-next tree (through v3.17-next/cpuidle-exynos), so it should be fixed there. On a side note, I wonder if we should have platform dependencies at all, or just the MCPM dependency by itself. This was discussed in some other thread already but I'm too lazy to dig a reference to it. Only a dependency on MCPM alone is needed here. And then: Acked-by: Nicolas Pitre n...@linaro.org diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm index 2f6b33ea6e08..459b7c91407a 100644 --- a/drivers/cpuidle/Kconfig.arm +++ b/drivers/cpuidle/Kconfig.arm @@ -10,6 +10,7 @@ config ARM_ARMADA_370_XP_CPUIDLE config ARM_BIG_LITTLE_CPUIDLE bool Support for ARM big.LITTLE processors depends on ARCH_VEXPRESS_TC2_PM || ARCH_EXYNOS + depends on MCPM select ARM_CPU_SUSPEND select CPU_IDLE_MULTIPLE_DRIVERS help ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] ARM: EXYNOS: Use MCPM call-backs to support S2R on Exynos5420
On Fri, 4 Jul 2014, Abhilash Kesavan wrote:
On Fri, Jul 4, 2014 at 9:43 AM, Nicolas Pitre nicolas.pi...@linaro.org
wrote:
Another suggestion which might possibly be better: why not looking for
the SYS_PWR_CFG bit in exynos_cpu_power_down() directly? After all,
exynos_cpu_power_down() is semantically supposed to do what its name
suggest and could simply do nothing if the proper conditions are already
in place.
I have implemented this and it works fine. Patch coming up.
On Fri, 4 Jul 2014, Abhilash Kesavan wrote:
Use the MCPM layer to handle core suspend/resume on Exynos5420.
Also, restore the entry address setup code post-resume.
Signed-off-by: Abhilash Kesavan a.kesa...@samsung.com
---
Changes in v2:
- Made use of the MCPM suspend/powered_up call-backs
Changes in v3:
- Used the residency value to indicate the entered state
Changes in v4:
- Checked if MCPM has been enabled to prevent build error
Changes in v5:
- Removed the MCPM flags and just used a local flag to
indicate that we are suspending.
Changes in v6:
- Read the SYS_PWR_REG value to decide if we are suspending
the system.
- Restore the SYS_PWR_REG value post-resume.
- Modified the comments to reflect the first change.
[...]
@@ -150,7 +153,15 @@ static void exynos_power_down(void)
BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
cpu_use_count[cpu][cluster]--;
if (cpu_use_count[cpu][cluster] == 0) {
- exynos_cpu_power_down(cpunr);
+ /*
+ * Bypass power down for CPU0 during suspend. Check for
+ * the SYS_PWR_REG value to decide if we are suspending
+ * the system.
+ */
+ temp = __raw_readl(pmu_base_addr +
+ EXYNOS5_ARM_CORE0_SYS_PWR_REG);
+ if ((cpu != 0) || ((temp S5P_CORE_LOCAL_PWR_EN) != 0))
+ exynos_cpu_power_down(cpunr);
Nah... We're going in circles, aren't we?
What I suggested above is:
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index 67d383de61..0a48421860 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -110,6 +110,16 @@ static int exynos_irq_set_wake(struct irq_data *data,
unsigned int state)
*/
void exynos_cpu_power_down(int cpu)
{
+ if (soc_is_exynos5250() cpu == 0) {
+ /*
+* Bypass power down for CPU0 during suspend. Check for
+* the SYS_PWR_REG value to decide if we are suspending
+* the system.
+*/
+ int val = __raw_readl(pmu_base_addr
+EXYNOS5_ARM_CORE0_SYS_PWR_REG);
+ if (!(val S5P_CORE_LOCAL_PWR_EN))
+ return;
+ }
__raw_writel(0, EXYNOS_ARM_CORE_CONFIGURATION(cpu));
}
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] ARM: EXYNOS: Use MCPM call-backs to support S2R on Exynos5420
On Sat, 5 Jul 2014, Abhilash Kesavan wrote: On a different note, I have been using the cpuidle patchset (https://patchwork.kernel.org/patch/4357421/) as base for S2R support and had a question. Rather than making the driver depend on ARCH_EXYNOS should it depend on EXYNOS5420_MCPM which in turn depends on MCPM (like TC2) or should we just be making the bL cpuidle driver depend on MCPM ? Probably making it depend on MCPM directly is the best approach. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v4] ARM: EXYNOS: Use MCPM call-backs to support S2R on Exynos5420
On Thu, 3 Jul 2014, Abhilash Kesavan wrote:
Use the MCPM layer to handle core suspend/resume on Exynos5420.
Also, restore the entry address setup code post-resume.
Signed-off-by: Abhilash Kesavan a.kesa...@samsung.com
---
Changes in v2:
- Made use of the MCPM suspend/powered_up call-backs
Changes in v3:
- Used the residency value to indicate the entered state
Changes in v4:
- Checked if MCPM has been enabled to prevent build error
This has been tested both on an SMDK5420 and Peach Pit Chromebook on
3.16-rc3/next-20140702.
Here are the dependencies (some of these patches did not apply cleanly):
1) Cleanup patches for mach-exynos
http://comments.gmane.org/gmane.linux.kernel.samsung-soc/33772
2) PMU cleanup and refactoring for using DT
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg671625.html
3) Exynos5420 PMU/S2R Series
http://comments.gmane.org/gmane.linux.kernel.samsung-soc/33898
4) MCPM boot CPU CCI enablement patches
www.spinics.net/lists/linux-samsung-soc/msg32923.html
5) Exynos5420 CPUIdle Series which populates MCPM suspend/powered_up
call-backs.
www.gossamer-threads.com/lists/linux/kernel/1945347
https://patchwork.kernel.org/patch/4357461/
6) Exynos5420 MCPM cluster power down support
http://www.spinics.net/lists/arm-kernel/msg339988.html
7) TPM reset mask patch
http://www.spinics.net/lists/arm-kernel/msg341884.html
arch/arm/include/asm/mcpm.h |6
arch/arm/mach-exynos/mcpm-exynos.c | 50
--
arch/arm/mach-exynos/pm.c| 38 --
arch/arm/mach-exynos/regs-pmu.h |1 +
drivers/cpuidle/cpuidle-big_little.c |2 +-
5 files changed, 79 insertions(+), 18 deletions(-)
diff --git a/arch/arm/include/asm/mcpm.h b/arch/arm/include/asm/mcpm.h
index ff73aff..051fbf1 100644
--- a/arch/arm/include/asm/mcpm.h
+++ b/arch/arm/include/asm/mcpm.h
@@ -272,4 +272,10 @@ void __init mcpm_smp_set_ops(void);
#define MCPM_SYNC_CLUSTER_SIZE \
(MCPM_SYNC_CLUSTER_INBOUND + __CACHE_WRITEBACK_GRANULE)
+/* Definitions for various MCPM scenarios that might need special handling */
+#define MCPM_CPU_IDLE0x0
+#define MCPM_CPU_SUSPEND 0x1
+#define MCPM_CPU_SWITCH 0x2
+#define MCPM_CPU_HOTPLUG 0x3
Please, let's avoid going that route. There is no such special handling
needed if the API is sufficient. And the provided API allows you to
suspend a CPU or shut it down. It shouldn't matter at that level if
this is due to a cluster switch or a hotplug event. Do you really need
something else?
[...]
@@ -129,7 +132,7 @@ static int exynos_power_up(unsigned int cpu, unsigned int
cluster)
* and can only be executed on processors like A15 and A7 that hit the cache
* with the C bit clear in the SCTLR register.
*/
-static void exynos_power_down(void)
+static void exynos_mcpm_power_down(u64 residency)
{
unsigned int mpidr, cpu, cluster;
bool last_man = false, skip_wfi = false;
@@ -150,7 +153,12 @@ static void exynos_power_down(void)
BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
cpu_use_count[cpu][cluster]--;
if (cpu_use_count[cpu][cluster] == 0) {
- exynos_cpu_power_down(cpunr);
+ /*
+ * Bypass power down for CPU0 during suspend. This is being
+ * taken care by the SYS_PWR_CFG bit in CORE0_SYS_PWR_REG.
+ */
+ if ((cpunr != 0) || (residency != MCPM_CPU_SUSPEND))
+ exynos_cpu_power_down(cpunr);
if (exynos_cluster_unused(cluster)) {
exynos_cluster_power_down(cluster);
@@ -209,6 +217,11 @@ static void exynos_power_down(void)
/* Not dead at this point? Let our caller cope. */
}
+static void exynos_power_down(void)
+{
+ exynos_mcpm_power_down(MCPM_CPU_SWITCH | MCPM_CPU_HOTPLUG);
+}
To distinguish between a suspend and a power-down, you can simply use
exynos_power_down() as your common handler, and have
exynos_mcpm_power_down() and exynos_mcpm_suspend() as wrappers around it
passing the appropriate private flags with local meanings.
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: several messages
On Thu, 3 Jul 2014, Abhilash Kesavan wrote:
Hi Nicolas,
On Thu, Jul 3, 2014 at 9:15 PM, Nicolas Pitre nicolas.pi...@linaro.org
wrote:
On Thu, 3 Jul 2014, Abhilash Kesavan wrote:
On Thu, Jul 3, 2014 at 6:59 PM, Nicolas Pitre nicolas.pi...@linaro.org
wrote:
Please, let's avoid going that route. There is no such special handling
needed if the API is sufficient. And the provided API allows you to
suspend a CPU or shut it down. It shouldn't matter at that level if
this is due to a cluster switch or a hotplug event. Do you really need
something else?
No, just one local flag for suspend should be enough for me. Will remove
these.
[...]
Changes in v5:
- Removed the MCPM flags and just used a local flag to
indicate that we are suspending.
[...]
-static void exynos_power_down(void)
+static void exynos_mcpm_power_down(u64 residency)
{
unsigned int mpidr, cpu, cluster;
bool last_man = false, skip_wfi = false;
@@ -150,7 +153,12 @@ static void exynos_power_down(void)
BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
cpu_use_count[cpu][cluster]--;
if (cpu_use_count[cpu][cluster] == 0) {
- exynos_cpu_power_down(cpunr);
+ /*
+ * Bypass power down for CPU0 during suspend. This is being
+ * taken care by the SYS_PWR_CFG bit in CORE0_SYS_PWR_REG.
+ */
+ if ((cpunr != 0) || (residency != S5P_CHECK_SLEEP))
+ exynos_cpu_power_down(cpunr);
if (exynos_cluster_unused(cluster)) {
exynos_cluster_power_down(cluster);
@@ -209,6 +217,11 @@ static void exynos_power_down(void)
/* Not dead at this point? Let our caller cope. */
}
+static void exynos_power_down(void)
+{
+ exynos_mcpm_power_down(0);
+}
[...]
+static int notrace exynos_mcpm_cpu_suspend(unsigned long arg)
+{
+ /* MCPM works with HW CPU identifiers */
+ unsigned int mpidr = read_cpuid_mpidr();
+ unsigned int cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+ unsigned int cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+
+ __raw_writel(0x0, sysram_base_addr + EXYNOS5420_CPU_STATE);
+
+ mcpm_set_entry_vector(cpu, cluster, exynos_cpu_resume);
+
+ /*
+ * Pass S5P_CHECK_SLEEP flag to the MCPM back-end to indicate that
+ * we are suspending the system and need to skip CPU0 power down.
+ */
+ mcpm_cpu_suspend(S5P_CHECK_SLEEP);
NAK.
When I say local flag with local meaning, I don't want you to smuggle
that flag through a public interface either. I find it rather inelegant
to have the notion of special handling for CPU0 being spread around like
that.
If CPU0 is special, then it should be dealth with in one place only,
ideally in the MCPM backend, so the rest of the kernel doesn't have to
care.
Again, here's what I mean:
static void exynos_mcpm_down_handler(int flags)
{
[...]
if ((cpunr != 0) || !(flags SKIP_CPU_POWERDOWN_IF_CPU0))
exynos_cpu_power_down(cpunr);
[...]
}
static void exynos_mcpm_power_down()
{
exynos_mcpm_down_handler(0);
}
static void exynos_mcpm_suspend(u64 residency)
{
/*
* Theresidency argument is ignored for now.
* However, in the CPU suspend case, we bypass power down for
* CPU0 as this is being taken care by the SYS_PWR_CFG bit in
* CORE0_SYS_PWR_REG.
*/
exynos_mcpm_down_handler(SKIP_CPU_POWERDOWN_IF_CPU0);
}
And SKIP_CPU_POWERDOWN_IF_CPU0 is defined in and visible to
mcpm-exynos.c only.
Sorry if I am being dense, but the exynos_mcpm_suspend function would
get called from both the bL cpuidle driver as well the exynos pm code.
What is that exynos pm code actually doing?
We want to skip CPU0 only in case of the S2R case i.e. the pm code and
keep the CPU0 power down code for all other cases including CPUIdle.
OK. If so I missed that somehow (hint hint).
If I call exynos_mcpm_down_handler with the flag in
exynos_mcpm_suspend(), CPUIdle will also skip CPU0 isn't it ?
As it is, yes. You could logically use an infinite residency time
(something like U64_MAX) to distinguish S2RAM from other types of
suspend.
Yet, why is this SYS_PWR_CFG bit set outside of MCPM? Couldn't the MCPM
backend handle it directly instead of expecting some other entity to do
it?
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: several messages
On Fri, 4 Jul 2014, Abhilash Kesavan wrote:
Hi Nicolas,
On Fri, Jul 4, 2014 at 12:30 AM, Nicolas Pitre nicolas.pi...@linaro.org
wrote:
On Thu, 3 Jul 2014, Abhilash Kesavan wrote:
Hi Nicolas,
On Thu, Jul 3, 2014 at 9:15 PM, Nicolas Pitre nicolas.pi...@linaro.org
wrote:
On Thu, 3 Jul 2014, Abhilash Kesavan wrote:
On Thu, Jul 3, 2014 at 6:59 PM, Nicolas Pitre
nicolas.pi...@linaro.org wrote:
Please, let's avoid going that route. There is no such special
handling
needed if the API is sufficient. And the provided API allows you to
suspend a CPU or shut it down. It shouldn't matter at that level if
this is due to a cluster switch or a hotplug event. Do you really need
something else?
No, just one local flag for suspend should be enough for me. Will
remove these.
[...]
Changes in v5:
- Removed the MCPM flags and just used a local flag to
indicate that we are suspending.
[...]
-static void exynos_power_down(void)
+static void exynos_mcpm_power_down(u64 residency)
{
unsigned int mpidr, cpu, cluster;
bool last_man = false, skip_wfi = false;
@@ -150,7 +153,12 @@ static void exynos_power_down(void)
BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
cpu_use_count[cpu][cluster]--;
if (cpu_use_count[cpu][cluster] == 0) {
- exynos_cpu_power_down(cpunr);
+ /*
+ * Bypass power down for CPU0 during suspend. This is
being
+ * taken care by the SYS_PWR_CFG bit in CORE0_SYS_PWR_REG.
+ */
+ if ((cpunr != 0) || (residency != S5P_CHECK_SLEEP))
+ exynos_cpu_power_down(cpunr);
if (exynos_cluster_unused(cluster)) {
exynos_cluster_power_down(cluster);
@@ -209,6 +217,11 @@ static void exynos_power_down(void)
/* Not dead at this point? Let our caller cope. */
}
+static void exynos_power_down(void)
+{
+ exynos_mcpm_power_down(0);
+}
[...]
+static int notrace exynos_mcpm_cpu_suspend(unsigned long arg)
+{
+ /* MCPM works with HW CPU identifiers */
+ unsigned int mpidr = read_cpuid_mpidr();
+ unsigned int cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+ unsigned int cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+
+ __raw_writel(0x0, sysram_base_addr + EXYNOS5420_CPU_STATE);
+
+ mcpm_set_entry_vector(cpu, cluster, exynos_cpu_resume);
+
+ /*
+ * Pass S5P_CHECK_SLEEP flag to the MCPM back-end to indicate that
+ * we are suspending the system and need to skip CPU0 power down.
+ */
+ mcpm_cpu_suspend(S5P_CHECK_SLEEP);
NAK.
When I say local flag with local meaning, I don't want you to smuggle
that flag through a public interface either. I find it rather inelegant
to have the notion of special handling for CPU0 being spread around like
that.
If CPU0 is special, then it should be dealth with in one place only,
ideally in the MCPM backend, so the rest of the kernel doesn't have to
care.
Again, here's what I mean:
static void exynos_mcpm_down_handler(int flags)
{
[...]
if ((cpunr != 0) || !(flags SKIP_CPU_POWERDOWN_IF_CPU0))
exynos_cpu_power_down(cpunr);
[...]
}
static void exynos_mcpm_power_down()
{
exynos_mcpm_down_handler(0);
}
static void exynos_mcpm_suspend(u64 residency)
{
/*
* Theresidency argument is ignored for now.
* However, in the CPU suspend case, we bypass power down for
* CPU0 as this is being taken care by the SYS_PWR_CFG bit in
* CORE0_SYS_PWR_REG.
*/
exynos_mcpm_down_handler(SKIP_CPU_POWERDOWN_IF_CPU0);
}
And SKIP_CPU_POWERDOWN_IF_CPU0 is defined in and visible to
mcpm-exynos.c only.
Sorry if I am being dense, but the exynos_mcpm_suspend function would
get called from both the bL cpuidle driver as well the exynos pm code.
What is that exynos pm code actually doing?
exynos pm code is shared across Exynos4 and 5 SoCs. It primarily does
a series of register configurations which are required to put the
system to sleep (some parts of these are SoC specific and others
common). It also populates the suspend_ops for exynos. In the current
patch, exynos_suspend_enter() is where I have plugged in the
mcpm_cpu_suspend call.
This patch is based on the S2R series for 5420
(http://comments.gmane.org/gmane.linux.kernel.samsung-soc/33898), you
may also have a look at that for a clearer picture.
We want to skip CPU0 only in case of the S2R case i.e. the pm code and
keep the CPU0 power down code for all other cases including CPUIdle.
OK. If so I missed that somehow (hint hint).
If I call exynos_mcpm_down_handler with the flag
Re: [PATCH v2] ARM: EXYNOS: Use MCPM call-backs to support S2R on Exynos5420
On Tue, 1 Jul 2014, Lorenzo Pieralisi wrote:
On Tue, Jul 01, 2014 at 02:14:49PM +0100, Abhilash Kesavan wrote:
Hi Nicolas,
On Tue, Jul 1, 2014 at 9:49 AM, Nicolas Pitre nicolas.pi...@linaro.org
wrote:
On Mon, 30 Jun 2014, Abhilash Kesavan wrote:
Use the MCPM layer to handle core suspend/resume on Exynos5420.
Also, restore the entry address setup code post-resume.
Signed-off-by: Abhilash Kesavan a.kesa...@samsung.com
---
[...]
Could you tell me more about this?
@@ -150,7 +153,13 @@ static void exynos_power_down(void)
BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
cpu_use_count[cpu][cluster]--;
if (cpu_use_count[cpu][cluster] == 0) {
- exynos_cpu_power_down(cpunr);
+ /*
+ * Bypass power down for CPU0 during suspend. This is being
+ * taken care by the SYS_PWR_CFG bit in CORE0_SYS_PWR_REG.
+ */
+ if ((cpunr != 0) ||
+ (__raw_readl(pmu_base_addr + S5P_INFORM1) !=
S5P_CHECK_SLEEP))
+ exynos_cpu_power_down(cpunr);
What happens if CPU0 is the first in the cluster to be idled? Will it
remain powered up until all the others have gone idle too?
This check is only for handling the S2R CPU0 case. In case of
idle/switching the S5P_CHECK_SLEEP flag would not be set and hence
there would be no change in behavior for them.
During suspend, we enable the ARM_USE_STANDBY_WFI0 bit which tells the
PMU when the CPU0 is ready to be suspended. This in conjunction with
the sleep state core configuration (setting SYS_PWR_REG low) causes
the CPU0 to go down. We should not write to the CPU0 power
configuration register (exynos_cpu_power_down) along with this during
suspend.
I think this should be part of a refactoring that includes the exynos MCPM
suspend call parameters. In particular, at the moment there is no code in
the back-end to detect if the last man should request core gating or cluster
gating (ie last man _always_ request cluster gating, that might not be what
we want), there is the residency value that can be also be used to imply a
S2R request (eg residency = ~0 ?).
The command sent must be implied by the state that is entered, not by
peeking at registers that should contain magic values, and that's true
not only for exynos but for all ARM platforms out there.
What I mean is: we can pass the requested state as a suspend parameter
and the power_down state machine will send the required command
accordingly.
It can be done using the residency value or by passing the power state index
as suspend parameter, the power down MCPM code will do a look-up and send
the respective command.
Thoughts appreciated.
I agree. Having the MCPM abstraction having to rely on some magic value
to be set in a register beforehand for things to work properly is not
nice.
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: convert all mov.* pc, reg to bx reg for ARMv6+ (part1)
On Tue, 1 Jul 2014, Will Deacon wrote: Hi Mans, On Tue, Jul 01, 2014 at 06:24:43PM +0100, Måns Rullgård wrote: Russell King - ARM Linux li...@arm.linux.org.uk writes: As you point out, bx lr /may/ be treated specially (I've actually been Most, if not all, Cortex-A cores do this according the public TRMs. They also do the same thing for mov pc, lr so there will probably be no performance gain from this change. It's still a good idea though, since we don't know what future cores will do. Funnily enough, that's not actually true (and is more or less what prompted this patch after discussion with Russell). There are cores out there that don't predict mov pc, lr at all (let alone do anything with the return stack). discussing this with Will Deacon over the last couple of days, who has also been talking to the hardware people in ARM, and Will is happy with this patch as in its current form.) This is why I've changed all mov pc, reg instructions which return in some way to use this macro, and left others (those which are used to call some function and return back to the same point) alone. In that case the patch should be fine. Your patch description didn't make it clear that only actual returns were being changed. I'm led to believe that some predictors require lr in order to update the return stack, whilst others don't. That part is all horribly micro-architectural, so the current patch is doing the right thing by sticking to the ARM ARM but enabling us to hook into other registers later on if we choose. May I suggest to have a patch with only the macro definition in it and all this discussion in the commit log please? The usage sites should be done in a separate patch to make it clearer. Nicolas
Re: [PATCH v2] ARM: EXYNOS: Use MCPM call-backs to support S2R on Exynos5420
On Mon, 30 Jun 2014, Abhilash Kesavan wrote:
Use the MCPM layer to handle core suspend/resume on Exynos5420.
Also, restore the entry address setup code post-resume.
Signed-off-by: Abhilash Kesavan a.kesa...@samsung.com
---
[...]
Could you tell me more about this?
@@ -150,7 +153,13 @@ static void exynos_power_down(void)
BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
cpu_use_count[cpu][cluster]--;
if (cpu_use_count[cpu][cluster] == 0) {
- exynos_cpu_power_down(cpunr);
+ /*
+ * Bypass power down for CPU0 during suspend. This is being
+ * taken care by the SYS_PWR_CFG bit in CORE0_SYS_PWR_REG.
+ */
+ if ((cpunr != 0) ||
+ (__raw_readl(pmu_base_addr + S5P_INFORM1) != S5P_CHECK_SLEEP))
+ exynos_cpu_power_down(cpunr);
What happens if CPU0 is the first in the cluster to be idled? Will it
remain powered up until all the others have gone idle too?
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: EXYNOS: Restore the entry address setup code post-resume
On Thu, 26 Jun 2014, Abhilash Kesavan wrote:
Hi,
On Thu, Jun 26, 2014 at 4:28 PM, Abhilash Kesavan a.kesa...@samsung.com
wrote:
Setup the mcpm entry address again on system resume as the
iRAM contents are lost across an s2r cycle.
Signed-off-by: Abhilash Kesavan a.kesa...@samsung.com
---
This has been tested after applying the Exynos5420 S2R support series
along with Nicolas Pitre's boot cluster CCI enablement patches on Peach
Pit.
arch/arm/mach-exynos/mcpm-exynos.c | 31 +--
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-exynos/mcpm-exynos.c
b/arch/arm/mach-exynos/mcpm-exynos.c
index 8c610e2..0bf734d 100644
--- a/arch/arm/mach-exynos/mcpm-exynos.c
+++ b/arch/arm/mach-exynos/mcpm-exynos.c
@@ -15,6 +15,7 @@
#include linux/delay.h
#include linux/io.h
#include linux/of_address.h
+#include linux/syscore_ops.h
#include asm/cputype.h
#include asm/cp15.h
@@ -26,6 +27,7 @@
#define EXYNOS5420_CPUS_PER_CLUSTER4
#define EXYNOS5420_NR_CLUSTERS 2
+static void __iomem *ns_sram_base_addr;
/*
* The common v7_exit_coherency_flush API could not be used because of the
* Erratum 799270 workaround. This macro is the same as the common one (in
@@ -308,10 +310,26 @@ static const struct of_device_id
exynos_dt_mcpm_match[] = {
{},
};
+static void exynos_mcpm_setup_entry_point(void)
+{
+ /*
+* U-Boot SPL is hardcoded to jump to the start of ns_sram_base_addr
+* as part of secondary_cpu_start(). Let's redirect it to the
+* mcpm_entry_point(). This is done during both secondary boot-up as
+* well as system resume.
+*/
+ __raw_writel(0xe59f, ns_sram_base_addr); /* ldr r0, [pc,
#0] */
+ __raw_writel(0xe12fff10, ns_sram_base_addr + 4); /* bx r0 */
+ __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 8);
+}
+
+static struct syscore_ops exynos_mcpm_syscore_ops = {
+ .resume = exynos_mcpm_setup_entry_point,
+};
+
static int __init exynos_mcpm_init(void)
{
struct device_node *node;
- void __iomem *ns_sram_base_addr;
int ret;
node = of_find_matching_node(NULL, exynos_dt_mcpm_match);
@@ -357,16 +375,9 @@ static int __init exynos_mcpm_init(void)
pr_info(Exynos MCPM support installed\n);
- /*
-* U-Boot SPL is hardcoded to jump to the start of ns_sram_base_addr
-* as part of secondary_cpu_start(). Let's redirect it to the
-* mcpm_entry_point().
-*/
- __raw_writel(0xe59f, ns_sram_base_addr); /* ldr r0, [pc,
#0] */
- __raw_writel(0xe12fff10, ns_sram_base_addr + 4); /* bx r0 */
- __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 8);
+ exynos_mcpm_setup_entry_point();
- iounmap(ns_sram_base_addr);
+ register_syscore_ops(exynos_mcpm_syscore_ops);
return ret;
}
This patch alone is not enough to bring the 8 cores back up post
resume on exynos5420, We need to enable the boot cluster snoops as
well.
Nicolas, if I add code to enable the boot cluster CCI in
mach-exynos/sleep.S, all 8 cores come up. However, if I use
mcpm_loopback as part of the newly added resume function to do the
same, I get a hang on resume.
Is there anything else that I need to take care of while doing this ?
No no no.
Forget about the MCPM loopback -- that's for boot time setup of the
booting cluster nothing else.
The snoops are enabled in a ***race free way*** by the low-level MCPM
code. But of course you must call it upon resume. Nothing else than
MCPM should ever play with the snoops enable/disable. Certainly not
mach-exynos/sleep.S.
You must tell your firmware to resume execution after suspend at
mcpm_entry_point. Before suspending, you tell MCPM where it should
branch to after it is done with its resuming business by calling
mcpm_set_entry_vector(), passing the address for exynos_cpu_resume for
example.
And of course the CPU shutdown during idle has to go through MCPM too.
When resumed, you also have to call mcpm_cpu_powered_up().
See how it is done in cpuidle-big_little.c for example.
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: exynos: activate the CCI on boot CPU/cluster with the MCPM loopback
On Tue, 24 Jun 2014, Doug Anderson wrote: Thank you very much for posting! With your series I'm able to boot all 8 cores on exynos5420-peach-pit and exynos5800-peach-pi sitting on my desk. Tested-by: Doug Anderson diand...@chromium.org Thanks to all. I've submitted those patches, with minor nits fixed, to RMK's system as patches 8081 to 8083. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] MCPM: clarify boot CPU situation wrt CCI
For the background story, please see: http://news.gmane.org/group/gmane.linux.kernel.samsung-soc/thread=32807 I sat on those patches for a while but they are the best I could think of in terms of implementation. To ease merging I suggest I collect all the ACK's and Tested-by's and submit them all at once to RMK's patch system. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] ARM: MCPM: provide infrastructure to allow for MCPM loopback
The kernel already has the responsibility to handle resources such as the
CCI when hotplugging CPUs, during the booting of secondary CPUs, and when
resuming from suspend/idle. It would be more coherent and less confusing
if the CCI for the boot CPU (or cluster) was also initialized by the kernel
rather than expecting the
firmware/bootloader to do it and only in that case. After all, the kernel
has all the necessary code already and the bootloader shouldn't have to
care at all.
The CCI may be turned on only when the cache is off. Leveraging the CPU
suspend code to loop back through the low-level MCPM entry point is all
that is needed to properly turn on the CCI from the kernel by using the
same code as for secondary boot.
Let's provide a generic MCPM loopback function that can be invoked by
backend initialization code to set things (CCI or similar) on the boot
CPU just as it is done for the other CPUs.
Signed-off-by: Nicolas Pitre n...@linaro.org
---
arch/arm/common/mcpm_entry.c | 52
arch/arm/include/asm/mcpm.h | 16 ++
2 files changed, 68 insertions(+)
diff --git a/arch/arm/common/mcpm_entry.c b/arch/arm/common/mcpm_entry.c
index f91136ab44..5e7284a3f8 100644
--- a/arch/arm/common/mcpm_entry.c
+++ b/arch/arm/common/mcpm_entry.c
@@ -12,11 +12,13 @@
#include linux/kernel.h
#include linux/init.h
#include linux/irqflags.h
+#include linux/cpu_pm.h
#include asm/mcpm.h
#include asm/cacheflush.h
#include asm/idmap.h
#include asm/cputype.h
+#include asm/suspend.h
extern unsigned long mcpm_entry_vectors[MAX_NR_CLUSTERS][MAX_CPUS_PER_CLUSTER];
@@ -146,6 +148,56 @@ int mcpm_cpu_powered_up(void)
return 0;
}
+#ifdef CONFIG_ARM_CPU_SUSPEND
+
+static int __init nocache_trampoline(unsigned long _arg)
+{
+ void (*cache_disable)(void) = (void *)_arg;
+ unsigned int mpidr = read_cpuid_mpidr();
+ unsigned int cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+ unsigned int cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+ phys_reset_t phys_reset;
+
+ mcpm_set_entry_vector(cpu, cluster, cpu_resume);
+ setup_mm_for_reboot();
+
+ __mcpm_cpu_going_down(cpu, cluster);
+ BUG_ON(!__mcpm_outbound_enter_critical(cpu, cluster));
+ cache_disable();
+ __mcpm_outbound_leave_critical(cluster, CLUSTER_DOWN);
+ __mcpm_cpu_down(cpu, cluster);
+
+ phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+ phys_reset(virt_to_phys(mcpm_entry_point));
+ BUG();
+}
+
+int __init mcpm_loopback(void (*cache_disable)(void))
+{
+ int ret;
+
+ /*
+* We're going to soft-restart the current CPU through the
+* low-level MCPM code by leveraging the suspend/resume
+* infrastructure. Let's play it safe by using cpu_pm_enter()
+* in case the CPU init code path resets the VFP or similar.
+*/
+ local_irq_disable();
+ local_fiq_disable();
+ ret = cpu_pm_enter();
+ if (!ret) {
+ ret = cpu_suspend((unsigned long)cache_disable,
nocache_trampoline);
+ cpu_pm_exit();
+ }
+ local_fiq_enable();
+ local_irq_enable();
+ if (ret)
+ pr_err(%s returned %d\n, __func__, ret);
+ return ret;
+}
+
+#endif
+
struct sync_struct mcpm_sync;
/*
diff --git a/arch/arm/include/asm/mcpm.h b/arch/arm/include/asm/mcpm.h
index 94060adba1..ff73affd45 100644
--- a/arch/arm/include/asm/mcpm.h
+++ b/arch/arm/include/asm/mcpm.h
@@ -217,6 +217,22 @@ int __mcpm_cluster_state(unsigned int cluster);
int __init mcpm_sync_init(
void (*power_up_setup)(unsigned int affinity_level));
+/**
+ * mcpm_loopback - make a run through the MCPM low-level code
+ *
+ * @cache_disable: pointer to function performing cache disabling
+ *
+ * This exercises the MCPM machinery by soft resetting the CPU and branching
+ * to the MCPM low-level entry code before returning to the caller.
+ * The @cache_disable function must do the necessary cache disabling to
+ * let the regular kernel init code turn it back on as if the CPU was
+ * hotplugged in. The MCPM state machine is set as if the cluster was
+ * initialized meaning the power_up_setup callback passed to mcpm_sync_init()
+ * will be invoked for all affinity levels. This may be useful to initialize
+ * some resources such as enabling the CCI that requires the cache to be off,
or simply for testing purposes.
+ */
+int __init mcpm_loopback(void (*cache_disable)(void));
+
void __init mcpm_smp_set_ops(void);
#else
--
1.8.4.108.g55ea5f6
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] ARM: exynos: activate the CCI on boot CPU/cluster with the MCPM loopback
The Chromebook firmware doesn't enable the CCI for the boot cpu, and
arguably it shouldn't have to either. Let's have the kernel handle the
CCI on its own for the boot CPU the same way it does it for secondary CPUs
by using the MCPM loopback.
Signed-off-by: Nicolas Pitre n...@linaro.org
---
arch/arm/mach-exynos/mcpm-exynos.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/mach-exynos/mcpm-exynos.c
b/arch/arm/mach-exynos/mcpm-exynos.c
index 0498d0b887..0c839f94ec 100644
--- a/arch/arm/mach-exynos/mcpm-exynos.c
+++ b/arch/arm/mach-exynos/mcpm-exynos.c
@@ -290,6 +290,19 @@ static void __naked exynos_pm_power_up_setup(unsigned int
affinity_level)
b cci_enable_port_for_self);
}
+static void __init exynos_cache_off(void)
+{
+ if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A15) {
+ /* disable L2 prefetching on the Cortex-A15 */
+ asm volatile(
+ mcrp15, 1, %0, c15, c0, 3\n\t
+ isb\n\t
+ dsb
+ : : r (0x400));
+ }
+ exynos_v7_exit_coherency_flush(all);
+}
+
static const struct of_device_id exynos_dt_mcpm_match[] = {
{ .compatible = samsung,exynos5420 },
{ .compatible = samsung,exynos5800 },
@@ -333,6 +346,8 @@ static int __init exynos_mcpm_init(void)
ret = mcpm_platform_register(exynos_power_ops);
if (!ret)
ret = mcpm_sync_init(exynos_pm_power_up_setup);
+ if (!ret)
+ ret = mcpm_loopback(exynos_cache_off); /* turn on the CCI */
if (ret) {
iounmap(ns_sram_base_addr);
return ret;
--
1.8.4.108.g55ea5f6
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] ARM: TC2: test the MCPM loopback during boot
This is not strictly needed on TC2 but still a good thing to exercise
that code.
Signed-off-by: nicolas Pitre n...@linaro.org
---
arch/arm/mach-vexpress/tc2_pm.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/arch/arm/mach-vexpress/tc2_pm.c b/arch/arm/mach-vexpress/tc2_pm.c
index b743a0ae02..54a9fff77c 100644
--- a/arch/arm/mach-vexpress/tc2_pm.c
+++ b/arch/arm/mach-vexpress/tc2_pm.c
@@ -323,6 +323,21 @@ static void __naked tc2_pm_power_up_setup(unsigned int
affinity_level)
b cci_enable_port_for_self );
}
+static void __init tc2_cache_off(void)
+{
+ pr_info(TC2: disabling cache during MCPM loopback test\n);
+ if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A15) {
+ /* disable L2 prefetching on the Cortex-A15 */
+ asm volatile(
+ mcrp15, 1, %0, c15, c0, 3 \n\t
+ isb\n\t
+ dsb
+ : : r (0x400) );
+ }
+ v7_exit_coherency_flush(all);
+ cci_disable_port_by_cpu(read_cpuid_mpidr());
+}
+
static int __init tc2_pm_init(void)
{
int ret, irq;
@@ -370,6 +385,8 @@ static int __init tc2_pm_init(void)
ret = mcpm_platform_register(tc2_pm_power_ops);
if (!ret) {
mcpm_sync_init(tc2_pm_power_up_setup);
+ /* test if we can (re)enable the CCI on our own */
+ BUG_ON(mcpm_loopback(tc2_cache_off) != 0);
pr_info(TC2 power management initialized\n);
}
return ret;
--
1.8.4.108.g55ea5f6
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: exynos: Support cluster power off on exynos5420/5800
On Wed, 18 Jun 2014, Abhilash Kesavan wrote:
Turning off a cluster when all 4 cores of the cluster are powered off
saves power significantly. Powering off the A15 L2 alone gives around
100mW in savings. Add support for powering off the A15/A7 clusters on
exynos5420/5800.
The patch enables specific register bits which ensure that:
- cluster L2 will be turned on before the first man is powered up.
- last man will be turned off before the cluster L2 is turned off.
- core is powered down before powering it up.
Remove the exynos_cluster_power_control function completely as we can
rely on the above mentioned bits rather than polling the cluster power
status register.
Signed-off-by: Abhilash Kesavan a.kesa...@samsung.com
Minor nit below:
@@ -342,6 +318,26 @@ static int __init exynos_mcpm_init(void)
pr_info(Exynos MCPM support installed\n);
/*
+ * On Exynos5420/5800 for the A15 and A7 clusters:
+ *
+ * EXYNOS5420_ENABLE_AUTOMATIC_CORE_DOWN ensures that all the cores
+ * in a cluster are turned off before turning off the cluster L2.
+ *
+ * EXYNOS5420_USE_ARM_CORE_DOWN_STATE ensures that a cores is powered
+ * off before waking it up.
+ *
+ * EXYNOS5420_USE_L2_COMMON_UP_STATE ensures that cluster L2 will be
+ * turned on before the first man is powered up.
+ */
+ for (i = 0; i EXYNOS5420_NR_CLUSTERS; i++) {
+ value = __raw_readl(EXYNOS_COMMON_CONFIGURATION(i) + 0x8);
+ value |= EXYNOS5420_ENABLE_AUTOMATIC_CORE_DOWN |
+ EXYNOS5420_USE_ARM_CORE_DOWN_STATE|
+ EXYNOS5420_USE_L2_COMMON_UP_STATE;
+ __raw_writel(value, EXYNOS_COMMON_CONFIGURATION(i) + 0x8);
Surely you can add another define in mach-exynos/regs-pmu.h to better
identify this register instead of EXYNOS_COMMON_CONFIGURATION()+8 ?
After that, you may add:
Acked-by: Nicolas Pitre n...@linaro.org
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ARM: EXYNOS: mcpm: Don't rely on firmware's secondary_cpu_start
On Mon, 16 Jun 2014, Doug Anderson wrote: Nicolas, On Mon, Jun 9, 2014 at 1:55 PM, Nicolas Pitre nicolas.pi...@linaro.org wrote: On Mon, 9 Jun 2014, Kevin Hilman wrote: On Mon, Jun 9, 2014 at 1:22 PM, Nicolas Pitre nicolas.pi...@linaro.org wrote: On Mon, 9 Jun 2014, Andrew Bresticker wrote: [1] While waiting for the forth-coming patch from Andrew to enable the CCI port for the boot cluster), I do this from u-boot before starting the kernel (based on earlier email from Doug): mw.l 10d25000 3 # Enable CCI from U-Boot From the other thread, it sounds like Nicolas wants enabling of the boot cluster's CCI port to be done unconditionally for all MCPM platforms. Nicolas, are you preparing a patch for this or should I? I would like confirmation this indeed solves your problem first before I go ahead with it. FYI... I confirmed on the other thread. It works on the Chromebook2. OK I'll clean it up for mainline. I just wanted to double-check that this is still on your list of things to do. If not then please let us know! :) It is. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: EXYNOS: mcpm: Don't rely on firmware's secondary_cpu_start
On Fri, 13 Jun 2014, Chander Kashyap wrote: This patch is effectively changing the mcpm_entry_point address from nsbase + 0x1c to nsbase + 0x8 Hence while integrating with mainline u-boot we need to take care for new mcpm_entry_point address. Why not inserting a NOP as the first instruction then? That way the offset will remain the same. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: EXYNOS: mcpm: Don't rely on firmware's secondary_cpu_start
On Tue, 10 Jun 2014, Doug Anderson wrote: My S-state knowledge is not strong, but I believe that Lorenzo's questions matter if we're using S2 for CPUidle (where we actually turn off power and hot unplug CPUs) but not when we're using S1 for CPUidle (where we just enter WFI/WFE). I believe that in ChromeOS we use S1 CPUidle and that it works fine. We've never implemented S2 that I'm aware of. You'll have to rely on MCPM for that. That's probably why it hasn't been implemented before. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems booting exynos5420 with 1 CPU
On Tue, 10 Jun 2014, Catalin Marinas wrote: Hi Nico, Sorry, I can't stay away from this thread ;) ;-) On Tue, Jun 10, 2014 at 12:25:47AM -0400, Nicolas Pitre wrote: On Mon, 9 Jun 2014, Lorenzo Pieralisi wrote: 4) When I am talking about firmware I am talking about sequences that are very close to HW (disabling C bit, cleaning caches, exiting coherency). Erratas notwithstanding, they are being standardized at ARM the best we can. They might even end up being implemented in HW in the not so far future. I understand they are tricky, I understand they take lots of time to implement them and to debug them, what I want to say is that they are becoming standard and we _must_ reuse the same code for all ARM platforms. You can implement them in MCPM (see (1)) or in firmware (and please do not start painting me as firmware hugger here, I am referring to standard power down sequences that again, are very close to HW state machines That's where the disconnect lies. On the one hand you say I understand they are tricky, I understand they take lots of time to implement them and to debug them and on the other hand you say They might end up being implemented in HW in the not so far future. That simply makes no economical sense at all! It makes lots of sense, though not from a software maintainability perspective. It would be nice if everything still looked like ARM7TDMI but in the race for performance (vs power), hardware becomes more complex and it's not just the CPU but adjacent parts like interconnects, caches, asynchronous bridges, voltage shifters, memory controllers, clocks/PLLs etc. Many of these are simply hidden from the high level OS like Linux because the OS assumes certain configuration (e.g. access to memory) and it's only the hardware itself that knows in what order they can be turned on or off (when triggered explicitly by the OS or an external event). I agree when the hardware has to handle parallel dependencies ordered in waterfall style. In such cases there is usually no point relying on software to implement what is nevertheless simple determinism with no possible alternative usage. But the *most* important thing is what you put in parents, so let me emphasize on what you just said: When triggered _explicitly_ by the OS or external events Having an dedicated power controller (e.g. M-class processor) to handle some of these is a rather flexible approach, other bits require RTL (and usually impossible to update). The M-class processor should be treated the same way as firmware. It ought to be flexible (certainly more than hardwired hardware), but it shares all the same downsides as firmware and the same concerns apply. When some operation is 1) tricky and takes time to debug, and 2) not performance critical (no one is trying to get in and out of idle or hibernation a billion times per second), then you should never ever put such a thing in firmware, and hardware should be completely out of the question! I agree that things can go wrong (both in hardware and software, no matter where it runs) but please don't think that such power architecture has been specifically engineered to hide the hardware from Linux. It's a necessity for complex systems and the optimal solution is not always simplification (it's not just ARM+vendors doing this, just look at the power model of modern x86 processors, hidden nicely from the software behind a few registers while making things harder for scheduler which cannot rely on a constant performance level; but it's a trade-off they are happy to make). I'll claim that this is a bad tradeoff. And the reason why some hardware architects might think it is a good one is because so far we really sucked at software based power management in Linux (and possibly other OSes as well). Hence the (fairly recent) realization that power management has to be integrated and under control of the scheduler rather than existing as some ad hoc subsystem. The reaction from the hardware people often is the software is crap and makes our hardware look bad, we know better, so let's make it easier on those poor software dudes by handling power management in hardware instead. But ultimately the hardware just can't predict things like software can. It might do a better job than the current software state of affairs, but most likely not be as efficient as a proper software architecture. The hardware may only be reactive, whereas the software can be proactive (when properly done that is). I sense from your paragraph above that ARM might be going the same direction as X86 and that would be very sad. Maybe the best compromise would be for all knobs to be made available to software if software wants to turn off the hardware auto-pilot and take control. This way the hardware guys would cover their arses while still allowing
Re: Problems booting exynos5420 with 1 CPU
On Tue, 10 Jun 2014, Catalin Marinas wrote: On Tue, Jun 10, 2014 at 05:49:01PM +0100, Nicolas Pitre wrote: The M-class processor should be treated the same way as firmware. It ought to be flexible (certainly more than hardwired hardware), but it shares all the same downsides as firmware and the same concerns apply. Yes, we can treat it as firmware, but we don't have a better alternative to move the functionality into the kernel (well, we could at least allow the kernel to load a binary blob and restart the controller). That would address the easy to update in the field side of the story. So far I've not seen this aspect been addressed with a serious plan anywhere. The reaction from the hardware people often is the software is crap and makes our hardware look bad, we know better, so let's make it easier on those poor software dudes by handling power management in hardware instead. But ultimately the hardware just can't predict things like software can. It might do a better job than the current software state of affairs, but most likely not be as efficient as a proper software architecture. The hardware may only be reactive, whereas the software can be proactive (when properly done that is). Indeed. But that's not the aim of the power controller on our boards. It's just a mechanism for safely changing sleep states, CPU frequencies but entirely under the OS decision. Sure. But then you might want to consider that some usage scenarios might benefit from the ability to abort a request, or monitor the progress of a request for software timing purposes, or accept parallel requests rather than serialize them, etc. Given the flexibility to extend beyond a rigid interface, the system may become even more efficient overall, albeit with added complexity in the implementation. But for that to work it has to be cheaply achievable. I sense from your paragraph above that ARM might be going the same direction as X86 and that would be very sad. Maybe the best compromise would be for all knobs to be made available to software if software wants to turn off the hardware auto-pilot and take control. This way the hardware guys would cover their arses while still allowing for the possibility that software might be able to out perform the hardware solution. I'm definitely not suggesting ARM is going the same route. Just trying to show that ARM is slightly better here. As a personal opinion, I like the simplicity of writing a register to change the P-state but I don't like the non-determinism of the x86 hardware w.r.t. CPU performance. There are however some policy aspects which I find interesting (like detecting whether the workload is memory bound and automatically lowering the CPU frequency; the OS cannot react this fast). This is not really a policy. This is a straight-forward waterfall dependency. There is simply nothing you can do with those CPU clock cycles when stalled most of the time waiting for memory queries to come back so the choice is obvious. If however this has a significant impact on code execution speed then this becomes a tradeoff and the choice to use this feature or not (the policy) must be implemented in software. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ARM: EXYNOS: mcpm: Don't rely on firmware's secondary_cpu_start
On Mon, 9 Jun 2014, Andrew Bresticker wrote: [1] While waiting for the forth-coming patch from Andrew to enable the CCI port for the boot cluster), I do this from u-boot before starting the kernel (based on earlier email from Doug): mw.l 10d25000 3 # Enable CCI from U-Boot From the other thread, it sounds like Nicolas wants enabling of the boot cluster's CCI port to be done unconditionally for all MCPM platforms. Nicolas, are you preparing a patch for this or should I? I would like confirmation this indeed solves your problem first before I go ahead with it. The only issue I see with making the MCPM loopback generic is that although all current mainline MCPM platforms have the same cache flush procedure, a future platform could be different. If you look at my patch the cache flush is factored out. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ARM: EXYNOS: mcpm: Don't rely on firmware's secondary_cpu_start
On Mon, 9 Jun 2014, Kevin Hilman wrote: On Mon, Jun 9, 2014 at 1:22 PM, Nicolas Pitre nicolas.pi...@linaro.org wrote: On Mon, 9 Jun 2014, Andrew Bresticker wrote: [1] While waiting for the forth-coming patch from Andrew to enable the CCI port for the boot cluster), I do this from u-boot before starting the kernel (based on earlier email from Doug): mw.l 10d25000 3 # Enable CCI from U-Boot From the other thread, it sounds like Nicolas wants enabling of the boot cluster's CCI port to be done unconditionally for all MCPM platforms. Nicolas, are you preparing a patch for this or should I? I would like confirmation this indeed solves your problem first before I go ahead with it. FYI... I confirmed on the other thread. It works on the Chromebook2. OK I'll clean it up for mainline. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems booting exynos5420 with 1 CPU
On Mon, 9 Jun 2014, Lorenzo Pieralisi wrote: I commented on Nico's patch because I did not like how it was implemented (at least remove the CPU PM notifier calls please, because they are not needed). OK no problem. That's easy enough. I added them to play it safe as a test patch in case some VFP content could be lost somehow by looping back through the CPU init code for example, and needed to be saved. I also said that's the only thing he could do, and I still think that's not a nice way to use the cpu_suspend API for something it was not designed for, that's what I wanted to say, period. Well... Maybe it wasn't designed for that, but it certainly can be used for that. And with no modifications to the core code, making this solution fairly elegant. This is not so different from, say, the BPF code being reused for seccomp_filters. BPF wasn't designed for system call filtering, but it happens to work well. If I am allowed to say something, here is a couple of thoughts. 1) CCI snoops and DVM enablement are secure operations, to do them in non-secure world this must be overriden in firmware. You can argue, you can think whatever you want, that's a fact. So, to use this code SMP bit in SCTLR and CCI enablement must become non-secure operations. This is a boot requirement for MCPM, right or wrong it is up to platform designers to judge. If CCI and SMP enablement are secure operations, we should not start adding random SMC calls in the kernel, since managing coherency in the kernel would become problematic, with lots of platform quirks. We do not want that to happen, and I think we all agree on this. One could certainly question the need for so many controls handled in secure world. But that is not the point. Here we're talking about MCPM. That implies the kernel has control over SCTLR.SMP and the CCI. If those things aren't under the kernel's control, then MCPM is of no use to you. Therefore, if you do want to use MCPM, then this implies the kernel has access to the CCI. And if it has access to it, then it should turn it on by itself in all cases to be consistent, not only in half of the cases. 2) (1) must be documented. Absolutely. But let's be coherent in the implementation so the documentation is as simple as it can be. 3) When I talked about consequences for CPUidle (implicit), I was referring to all sort of hacks we had to come up to bring devices like SPC (remember ? I remember very very well unfortunately for me), or whatever power controller up in the kernel early, too early to fit in any existing kernel device framework. There is still no solution to that, and the only way that code can exist is in mach- code. Right or wrong, that's a second fact and in my opinion that's not nice for the ARM kernel. I disagree. This can perfectly be turned into driver code. If we need it too early for existing kernel device framework to handle this properly, then the solution is to extend the existing framework or create another one specially for that purpose. This may not be obvious when TC2 is the first/only platform in that situation, but if more platforms have the same need then it'll be easier to abstract commonalities into a framework. Saying that no framework exists today or/and upstream maintainers are being difficult is _not_ a reason for throwing your hands up and e.g. shoving all this code into firmware instead. 4) When I am talking about firmware I am talking about sequences that are very close to HW (disabling C bit, cleaning caches, exiting coherency). Erratas notwithstanding, they are being standardized at ARM the best we can. They might even end up being implemented in HW in the not so far future. I understand they are tricky, I understand they take lots of time to implement them and to debug them, what I want to say is that they are becoming standard and we _must_ reuse the same code for all ARM platforms. You can implement them in MCPM (see (1)) or in firmware (and please do not start painting me as firmware hugger here, I am referring to standard power down sequences that again, are very close to HW state machines That's where the disconnect lies. On the one hand you say I understand they are tricky, I understand they take lots of time to implement them and to debug them and on the other hand you say They might end up being implemented in HW in the not so far future. That simply makes no economical sense at all! When some operation is 1) tricky and takes time to debug, and 2) not performance critical (no one is trying to get in and out of idle or hibernation a billion times per second), then you should never ever put such a thing in firmware, and hardware should be completely out of the question! and more importantly if they HAVE to run in secure world that's the only solution we have unless you want to split race
Re: Problems booting exynos5420 with 1 CPU
On Sun, 8 Jun 2014, Lorenzo Pieralisi wrote: On Sun, Jun 08, 2014 at 12:53:34AM +0100, Olof Johansson wrote: Lorenzo, Since you're emailing from @arm.com, some of this is to the wider recipient and maybe not directly to you: I am glad to reply and take blame since this is a debate definitely worth having. Great. Because I would like to steer this debate a little towards the genuine cause rather than sticking to some particular consequences. Guys, do not get me wrong here. There are fixes that can be deemed acceptable in an OS, there are fixes that can't. I just can't help thinking that Nicolas' patch is a nasty hack (and I am far, really really far from blaming him for that, because that's the only patch that can fix that issue in the kernel), and he perfectly knows that. You know what? The more I think about my patch, the more I consider this should be the standard way of setting up things unconditionally on _all_ platforms using MCPM. Why? Because that's the most coherent thing to do! I really think the kernel should either be responsible for the CCI or it should not at all. And conversely for the bootloader. Right now we have an implicit requirement that the bootloader should turn on the CCI, but only for cold boot, and only for the boot cluster, and not for CPU resuming from idle, and what other case we haven't thought about yet. And as noticed this requirement is not documented. Whatever the outcome of this thread, a booting protocol update for CCI is in order, even if we have to debate it for 6 months or more to get an agreement. And in the end I don't think the CCI should have to be documented as a boot requirement. Instead of having firmware implementers understand when they should care about the CCI and when they shouldn't, I'd much prefer if they hadn't to care at all. I really prefer when responsibility for something is well encapsulated in one place and not shared across layers like the firmware or the kernel depending on some context. The complexity of a system (and therefore the probability for bugs) grows with the square of the number of interrelations between constituent parts. So we should always strive to make the boot protocol _simpler_ not more complex. And if complete responsibility for the CCI in the kernel had been assumed from the beginning, we wouldn't be struggling in this power play to determine which side should give in. Especially since the kernel has all the necessary infrastructure to do it all already, and I must say in a rather elegant manner. I'm a very strong proponent of enabling upstream support for our platform (for several reasons -- most of these are actually business reasons for us, but also because it's the right thing to do). Finding the trade-off for what workarounds are still reasonable to do in the kernel for that situation is obviously hard and we're disagreeing. But the scope for these workarounds is not large. Will people ever realize that, if the kernel was more in control of the hardware (isn't that the role of an OS kernel to serve as the hardware abstraction layer?) then we wouldn't be talking about workarounds but rather standard fixes? In this case, the change we're looking at is enabling the CCI port for the boot cpu. It's perfectly containable in exynos-only code, and we can surround it by however many comments of never ever using it as an example for how to do it as you'll want. In this case, to state my opinion clearly, it is the general design that was flawed and the kernel should be fixed to enable the CCI for the boot CPU itself _when_ it knows it is going to need it. To start with, the bootloader has no need what so ever for using more than one CPU, unless it wants to become an operating system, so it shouldn't have to care at all. The kernel, if booted without the CCI information in the DTB, will run with only one CPU and won't rely on the CCI. Logically the CCI could be left turned off in that case, possibly increasing bus performance and saving some power. I agree with what you are saying, but if for any reason someone will copy that code to paper over yet another firmware quirk and think that's the right thing to do, that would be grave IMHO. Someone shouldn't have to copy that code because I'm getting more and more convinced it should be made generic and unconditional, and by doing so removing any possibility for firmware to get that part wrong again. According to my quick experiment on TC2, this took only 271 microseconds to perform so this is not like if that would make a significant difference in boot time. I do not think they do. The kernel should not become a place where firmware bugs are fixed, if you refuse to fix the bug and this code does not get upstream I am pretty sure next time more attention will be paid. Again, this is coming about because firmware is a MAGNITUDE harder to fix and IMPOSSIBLE
Re: Problems booting exynos5420 with 1 CPU
On Sun, 8 Jun 2014, Russell King - ARM Linux wrote: On Sat, Jun 07, 2014 at 04:53:34PM -0700, Olof Johansson wrote: You do realize that you have absolutely zero leverage over us on this, right? Our product is already shipped with kernel code that fixes this. That is never a justification for forcing /any/ code into the kernel. We've been here before with the iPAQs, where there were all sorts of horrid hacks that were in the code for that device, and we said no to it, and we kept it out of the mainline kernel, and stopped those hacks polluting elsewhere (because people got to know on the whole that if they used those hacks, it would bar them from mainline participation.) That's different. The iPaq had very little in terms of firmware, and the one it had was field upgradable with almost no risk to brick it (unless you wanted to hack the firmware code but that's another story). The reason iPaq had never been well supported in mainline can be attributed to laziness for not wanting to make the code into a shape acceptable for mainline inclusion. But nothing fundamentally prevented that from happening if someone had wanted to do it. Here we're talking about firmware-induced hacks for which, had there been no firmware, the kernel would be in full position to fix properly because that would have been a kernel bug after all. Hence my crusade against this you should abstract things in firmware mantra. Especially for mobile devices. But, because firmware already exists out there and it is between prohibitive and impossible to fix, we have no choice but to compensate in the kernel some times. In this very case my approach is to render the firmware irrelevant globally rather than trying to work around it for one particular device. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems booting exynos5420 with 1 CPU
On Sun, 8 Jun 2014, Russell King - ARM Linux wrote: On Sun, Jun 08, 2014 at 02:26:43PM -0400, Nicolas Pitre wrote: On Sun, 8 Jun 2014, Russell King - ARM Linux wrote: On Sat, Jun 07, 2014 at 04:53:34PM -0700, Olof Johansson wrote: You do realize that you have absolutely zero leverage over us on this, right? Our product is already shipped with kernel code that fixes this. That is never a justification for forcing /any/ code into the kernel. We've been here before with the iPAQs, where there were all sorts of horrid hacks that were in the code for that device, and we said no to it, and we kept it out of the mainline kernel, and stopped those hacks polluting elsewhere (because people got to know on the whole that if they used those hacks, it would bar them from mainline participation.) That's different. The iPaq had very little in terms of firmware, and the one it had was field upgradable with almost no risk to brick it (unless you wanted to hack the firmware code but that's another story). The reason iPaq had never been well supported in mainline can be attributed to laziness for not wanting to make the code into a shape acceptable for mainline inclusion. But nothing fundamentally prevented that from happening if someone had wanted to do it. No, I was specifically thinking about the various iPAQ specific things like the additional platform specific ATAGs that they invented with zero reference to mainline, and then expected them to be accepted as-is. I gave them a very clear message over that (which was no way) and that was essentially the last of their mainline submissions. That's basically what I'm saying. They were too lazy to fix their code. But nothing prevented that otherwise. They certainly couldn't use the firmware is broken and too hard to update excuse. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems booting exynos5420 with 1 CPU
On Sat, 7 Jun 2014, Abhilash Kesavan wrote:
Hi Nicolas,
The first man of the incoming cluster enables its snoops via the
power_up_setup function. During secondary boot-up, this does not occur
for the boot cluster. Hence, I enable the snoops for the boot cluster
as a one-time setup from the u-boot prompt. After secondary boot-up
there is no modification that I do.
OK that's good.
Where should this be ideally done ?
If I remember correctly, the CCI can be safely activated only when the
cache is disabled. So that means the CCI should ideally be turned on
for the boot cluster (and *only* for the boot CPU) by the bootloader.
Now... If you _really_ prefer to do it from the kernel to avoid
difficulties with bootloader updates, then it should be possible to do
it from the kernel by temporarily turning the cache off. This is not a
small thing but the MCPM infrastructure can be leveraged. Here's what I
tried on a TC2 which might just work for you as well:
diff --git a/arch/arm/common/mcpm_entry.c b/arch/arm/common/mcpm_entry.c
index 86fd60fefb..1cc49de308 100644
--- a/arch/arm/common/mcpm_entry.c
+++ b/arch/arm/common/mcpm_entry.c
@@ -12,11 +12,13 @@
#include linux/kernel.h
#include linux/init.h
#include linux/irqflags.h
+#include linux/cpu_pm.h
#include asm/mcpm.h
#include asm/cacheflush.h
#include asm/idmap.h
#include asm/cputype.h
+#include asm/suspend.h
extern unsigned long mcpm_entry_vectors[MAX_NR_CLUSTERS][MAX_CPUS_PER_CLUSTER];
@@ -146,6 +148,44 @@ int mcpm_cpu_powered_up(void)
return 0;
}
+static int go_down(unsigned long _arg)
+{
+ void (*cache_disable)(void) = (void *)_arg;
+ unsigned int mpidr = read_cpuid_mpidr();
+ unsigned int cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+ unsigned int cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+ phys_reset_t phys_reset;
+
+ mcpm_set_entry_vector(cpu, cluster, cpu_resume);
+ setup_mm_for_reboot();
+
+ __mcpm_cpu_going_down(cpu, cluster);
+ BUG_ON(!__mcpm_outbound_enter_critical(cpu, cluster));
+ cache_disable();
+ __mcpm_outbound_leave_critical(cluster, CLUSTER_DOWN);
+ __mcpm_cpu_down(cpu, cluster);
+
+ phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+ phys_reset(virt_to_phys(mcpm_entry_point));
+ BUG();
+}
+
+int mcpm_loopback(void (*cache_disable)(void))
+{
+ int ret;
+
+ local_irq_disable();
+ local_fiq_disable();
+ ret = cpu_pm_enter();
+ if (!ret) {
+ ret = cpu_suspend((unsigned long)cache_disable, go_down);
+ cpu_pm_exit();
+ }
+ local_fiq_enable();
+ local_irq_enable();
+ return ret;
+}
+
struct sync_struct mcpm_sync;
/*
diff --git a/arch/arm/mach-vexpress/tc2_pm.c b/arch/arm/mach-vexpress/tc2_pm.c
index 29e7785a54..abecdd734f 100644
--- a/arch/arm/mach-vexpress/tc2_pm.c
+++ b/arch/arm/mach-vexpress/tc2_pm.c
@@ -323,6 +323,26 @@ static void __naked tc2_pm_power_up_setup(unsigned int
affinity_level)
b cci_enable_port_for_self );
}
+int mcpm_loopback(void (*cache_disable)(void));
+
+static void tc2_cache_down(void)
+{
+ pr_warn(TC2: disabling cache\n);
+ if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A15) {
+ /*
+* On the Cortex-A15 we need to disable
+* L2 prefetching before flushing the cache.
+*/
+ asm volatile(
+ mcrp15, 1, %0, c15, c0, 3 \n\t
+ isb\n\t
+ dsb
+ : : r (0x400) );
+ }
+ v7_exit_coherency_flush(all);
+ cci_disable_port_by_cpu(read_cpuid_mpidr());
+}
+
static int __init tc2_pm_init(void)
{
int ret, irq;
@@ -370,6 +390,7 @@ static int __init tc2_pm_init(void)
ret = mcpm_platform_register(tc2_pm_power_ops);
if (!ret) {
mcpm_sync_init(tc2_pm_power_up_setup);
+ BUG_ON(mcpm_loopback(tc2_cache_down) != 0);
pr_info(TC2 power management initialized\n);
}
return ret;
Of course it is not because I'm helping you sidestepping firmware
problems that I'll stop shaming those who came up with that firmware
design. ;-)
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems booting exynos5420 with 1 CPU
On Sat, 7 Jun 2014, Lorenzo Pieralisi wrote: On Sat, Jun 07, 2014 at 05:10:27PM +0100, Nicolas Pitre wrote: On Sat, 7 Jun 2014, Abhilash Kesavan wrote: Hi Nicolas, The first man of the incoming cluster enables its snoops via the power_up_setup function. During secondary boot-up, this does not occur for the boot cluster. Hence, I enable the snoops for the boot cluster as a one-time setup from the u-boot prompt. After secondary boot-up there is no modification that I do. OK that's good. Where should this be ideally done ? If I remember correctly, the CCI can be safely activated only when the cache is disabled. So that means the CCI should ideally be turned on for the boot cluster (and *only* for the boot CPU) by the bootloader. CCI ports are enabled per-cluster, so the boot loader must turn on CCI for all clusters before the respective CPUs have a chance to turn on their caches. It is a secure operation, this can be overriden and probably that's what has been done, wrongly. Careful. By saying for all clusters you might be interpreted as saying that the CCI must be turned on even for CPUs that are not powered up. True, TC2 on warm-boot reenables CCI, and that's because it runs the kernel in secure world, and again that's _wrong_. Let me respectfully disagree. It must be done in firmware, and I am totally against any attempt at papering over what looks like a messed up firmware implementation with a bunch of hacks in the kernel, because that's what the patch below is (soft restarting a CPU to enable CCI ? and adding generic code for that ? what's next ?) Are you promoting for the removal of drivers/bus/arm-cci.c ? You do realize that the fundamental raison d'être for MCPM is actually to manage the race free enabling of the cache and CCI ? I understand there is an issue and lots at stake here, but I do not want the patch below to be merged in the kernel, I am sorry, it is a tad too much. Lorenzo: Don't get me wrong. The Chromebooks, and possibly to some extent some people at Samsung, were simply too confident in their ability to create absolutely bug-free firmware code to the point of not making its update easy in the field. This is completely outrageous in my point of view. Yet one of the reactions was to call upstream kernel people as purists because the kernel is so much easier to modify in order to cover their mess and yet that might not be accepted. Like I said I won't stop shaming them publicly for their own incompetence just yet (no pun intended), but being excessively purist does not benefit anyone either, and for that they have a point. *HOWEVER* I have no choice but to say that many people at ARM, including a couple individuals for whom I nevertheless have a lot of admiration, also have an incredible faith in their ability to convince themselves, and then turn around to preach to the world, that *more firmware* is going to be so much purer and solve so many more problems than it creates and become such a magical success that we should immediately dedicate our soul to the cause with no hint of a doubt. I'm sorry to rain on your parade, but I don't believe in this one iota. Let me repeat the MCPM story again: it took 3 people, including 2 from ARM, over *six* months to get everything right and stable on TC2. I think you also contributed to that effort as well. Subsequent MCPM backend contributions (yes, just the backend and not the core code) took at least *five* rounds of reviews in one case, and after *seven* rounds in another case it is still not right, despite the publicly available TC2 implementation to serve as a reference. I'm sure each time a new patch set was posted, their authors honestly believed their code was correct. Otherwise why would they post buggy code? Now you are telling me that they should have put that code into firmware instead? Can you realize what a catastrophe this would have been? Are you _seriously_ believing that they would be up to their 5th firmware revision by now? And that updating their firmware six months after product launch would be as easy as updating the kernel? Software ALWAYS has bugs, whether it is user apps, the kernel, firmware or boot ROM. The bigger one of those parts is, the more bugs it will have. And the cost to vendors for fixing those bugs grow exponentially down each level. For proof, we're now considering possible workarounds in the kernel to sidestep the difficulty with updating the firmware on a Chromebook. Yet you're saying that firmware should grow code with the same complexity as the MCPM core, plus a machine specific backend that experience has proven multiple times is evidently hard to get right, into firmware because running Linux in secure mode is wrong? If so we don't live in the same world indeed. The day I see a firmware architecture that allows for 1) the same level of peer review as what we enjoy
Re: Problems booting exynos5420 with 1 CPU
On Fri, 6 Jun 2014, Olof Johansson wrote: On Sat, Jun 07, 2014 at 02:16:27AM +0530, Abhilash Kesavan wrote: My answer is not use mainline u-boot primarily because I am not sure mainline u-boot actually works on 5420 :). And I'm saying that's not the answer primarily because we should never require people to update their firmware to get a usable linux system. My answer is keep a patch locally (or make a trivial change to the bootcmd) for people who would like to use an upstream kernel with the firmware on the device. Once we do have a working mainline u-boot, that can then be used by the interested parties. And I am strongly NAK:ing both of those approaches. We should not require a single out-of-tree patch because that means we have failed to make a useful kernel for people. And it should never, ever, be a requirement for people to reflash and risk bricking their device just to run mainline linux on it. What we can do, though, is to publicly shame you all Google People very strongly for not making firmware updates in the field safer and easier. After all you must all know already that, by definition, software always contains bugs. The first feature to be tested with a new bootloader/firmware must be the ability to successfully and safely (and securely) update itself. Especially for a mainstream device like a Chromebook. It's an artificial barrier of entry with high risk, and we'll be worse off for adding it. Same for out-of-tree patches. So ... let's find the lesser of all evils ... as always. iRAM is covered on Doug's sub-thread, and I think his approach looks promising. So, it seems like we have a solution both to enable the CCI port and to avoid clearing iram -- we should be set? Care to resume the proposed solution then? Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems booting exynos5420 with 1 CPU
On Fri, 6 Jun 2014, Abhilash Kesavan wrote: Hi Doug, The first change in the kernel (clearing an iRAM location) is needed because of an unnecessary change that we are carrying in the Chrome U-boot. There is no reason for us to have the workaround in the mainline kernel. Rather, we should remove the check from our u-boot. However AFAIR a clean-up patch that I had posted internally was not accepted as we had frozen the SPL at the time. The second change is to enable snoops for boot cluster. Internally, we were disabling the snoops for both the clusters at power off and enabling it in power_up_setup and power_up. However, I dropped the approach due to problems pointed out by Nicolas here http://www.spinics.net/lists/arm-kernel/msg324091.html related to cpuidle. Hence, we turn it on at the u-boot. You should never *ever* play with the CCI from U-Boot. That's for the MCPM layer to decide when it is safe to do so. That's mainly the _whole_ reason why MCPM exists in the first place. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: EXYNOS: mcpm: Don't rely on firmware's secondary_cpu_start
On Fri, 6 Jun 2014, Doug Anderson wrote: On exynos mcpm systems the firmware is hardcoded to jump to an address in SRAM (0x02073000) when secondary CPUs come up. By default the firmware puts a bunch of code at that location. That code expects the kernel to fill in a few slots with addresses that it uses to jump back to the kernel's entry point for secondary CPUs. Originally (on prerelease hardware) this firmware code contained a bunch of workarounds to deal with boot ROM bugs. However on all shipped hardware we simply use this code to redirect to a kernel function for bringing up the CPUs. Let's stop relying on the code provided by the bootloader and just plumb in our own (simple) code jump to the kernel. This has the nice benefit of fixing problems due to the fact that older bootloaders (like the one shipped on the Samsung Chromebook 2) might have put slightly different code into this location. Once suspend/resume is implemented for systems using exynos-mcpm we'll need to make sure we reinstall our fixed up code after resume. ...but that's not anything new since IRAM (and thus the address of the mcpm_entry_point) is lost across suspend/resume anyway. Signed-off-by: Doug Anderson diand...@chromium.org --- arch/arm/mach-exynos/mcpm-exynos.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c index 0498d0b..3a7fad0 100644 --- a/arch/arm/mach-exynos/mcpm-exynos.c +++ b/arch/arm/mach-exynos/mcpm-exynos.c @@ -343,11 +343,13 @@ static int __init exynos_mcpm_init(void) pr_info(Exynos MCPM support installed\n); /* - * Future entries into the kernel can now go - * through the cluster entry vectors. + * U-Boot SPL is hardcoded to jump to the start of ns_sram_base_addr + * as part of secondary_cpu_start(). Let's redirect it to the + * mcpm_entry_point(). */ - __raw_writel(virt_to_phys(mcpm_entry_point), - ns_sram_base_addr + MCPM_BOOT_ADDR_OFFSET); + __raw_writel(0xe59f, ns_sram_base_addr); /* ldr r0, [pc, #0] */ + __raw_writel(0xe12fff10, ns_sram_base_addr + 4); /* bx r0 */ + __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 8); Is this valid for all exynos systems, or is this particular to Chromebooks? If this is all that is needed to solve the problem being discussed in the other thread I have absolutely no issue with such a workaround going into mainline. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems booting exynos5420 with 1 CPU
On Fri, 6 Jun 2014, Doug Anderson wrote: Note that handling CPU resume in a way that can be updated by RW firmware is non-trivial and requires some SRAM to be saved across suspend/resume. Saved by the kernel or the firmware? Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ARM: EXYNOS: mcpm: Don't rely on firmware's secondary_cpu_start
On Fri, 6 Jun 2014, Doug Anderson wrote: On exynos mcpm systems the firmware is hardcoded to jump to an address in SRAM (0x02073000) when secondary CPUs come up. By default the firmware puts a bunch of code at that location. That code expects the kernel to fill in a few slots with addresses that it uses to jump back to the kernel's entry point for secondary CPUs. Originally (on prerelease hardware) this firmware code contained a bunch of workarounds to deal with boot ROM bugs. However on all shipped hardware we simply use this code to redirect to a kernel function for bringing up the CPUs. Let's stop relying on the code provided by the bootloader and just plumb in our own (simple) code jump to the kernel. This has the nice benefit of fixing problems due to the fact that older bootloaders (like the one shipped on the Samsung Chromebook 2) might have put slightly different code into this location. Once suspend/resume is implemented for systems using exynos-mcpm we'll need to make sure we reinstall our fixed up code after resume. ...but that's not anything new since IRAM (and thus the address of the mcpm_entry_point) is lost across suspend/resume anyway. Signed-off-by: Doug Anderson diand...@chromium.org Acked-by: Nicolas Pitre n...@linaro.org --- Changes in v2: - Removed #define arch/arm/mach-exynos/mcpm-exynos.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c index 0498d0b..ace0ed6 100644 --- a/arch/arm/mach-exynos/mcpm-exynos.c +++ b/arch/arm/mach-exynos/mcpm-exynos.c @@ -25,7 +25,6 @@ #define EXYNOS5420_CPUS_PER_CLUSTER 4 #define EXYNOS5420_NR_CLUSTERS 2 -#define MCPM_BOOT_ADDR_OFFSET0x1c /* * The common v7_exit_coherency_flush API could not be used because of the @@ -343,11 +342,13 @@ static int __init exynos_mcpm_init(void) pr_info(Exynos MCPM support installed\n); /* - * Future entries into the kernel can now go - * through the cluster entry vectors. + * U-Boot SPL is hardcoded to jump to the start of ns_sram_base_addr + * as part of secondary_cpu_start(). Let's redirect it to the + * mcpm_entry_point(). */ - __raw_writel(virt_to_phys(mcpm_entry_point), - ns_sram_base_addr + MCPM_BOOT_ADDR_OFFSET); + __raw_writel(0xe59f, ns_sram_base_addr); /* ldr r0, [pc, #0] */ + __raw_writel(0xe12fff10, ns_sram_base_addr + 4); /* bx r0 */ + __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 8); iounmap(ns_sram_base_addr); -- 2.0.0.526.g5318336 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: EXYNOS: Use wfi macro in platform_do_lowpower
On Thu, 22 May 2014, Daniel Lezcano wrote: On 05/22/2014 09:57 AM, Leela Krishna Amudala wrote: This patch is originally based on commit b3377d186572 (ARM: 7064/1: vexpress: Use wfi macro in platform_do_lowpower.) Current Exynos CPU hotplug code includes a hardcoded WFI instruction, in ARM encoding. When the kernel is compiled in Thumb-2 mode, this is invalid and causes the machine to hang hard when a CPU is offlined. Use wfi macro instead of the hardcoded WFI instruction. Is it possible to change it for cpu_do_idle() ? Please let's not encourage every wfi() to be turned into cpu_do_idle(). They are not always equivalent. And semantically they are not. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/5] MCPM backend for Exynos5420
On Mon, 19 May 2014, Abhilash Kesavan wrote:
Hi Nicolas,
On Thu, May 15, 2014 at 10:22 PM, Nicolas Pitre
nicolas.pi...@linaro.org wrote:
Once you implement full cluster shutdown I can provide you with another
script stressing that part.
I am done with the cluster power on/off code and it seems to work fine
with bL_switcher. Can you please provide me with your stress script.
Here's the script. Credits go to Dave Martin for this one.
The script assumes that cluster 0 comprises cpus 0 to 3 and cluster 1
comprises cpus 4 to 7. You need to edit it otherwise.
- 8
#!/bin/bash
echo 0 /sys/kernel/bL_switcher/active
sleep 1
pids=
for x in \
cluster=cluster0; cpus='cpu0 cpu1 cpu2 cpu3' \
cluster=cluster1; cpus='cpu4 cpu5 cpu6 cpu7'
do
eval $x
eval \
$cluster () {
pids=
`for cpu in $cpus; do cat EOF; done
(sleep .00\\$RANDOM; echo \\$1 /sys/devices/system/cpu/$cpu/online)
echo \\$1 /sys/devices/system/cpu/$cpu/online
pids=\\$pids\ \\$!
EOF`
wait \$pids
if [ \$1 = 0 ]; then
echo \\$n: $cluster off\
else
echo \\$n: $cluster on\
fi
}
done
n=0
while :; do
: $((++n))
cluster1 1; cluster0 0
cluster0 1; cluster1 0
done
- 8
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/5] MCPM backend for Exynos5420
On Thu, 15 May 2014, Abhilash Kesavan wrote: Hi Nicolas, On Wed, May 14, 2014 at 7:09 PM, Abhilash Kesavan kesavan.abhil...@gmail.com wrote: Hi Nicolas, On Wed, May 14, 2014 at 7:03 PM, Nicolas Pitre nicolas.pi...@linaro.org wrote: On Wed, 14 May 2014, Abhilash Kesavan wrote: Hi Nicolas, [...] 1) can't create /sys/devices/system/cpu/cpu//online: nonexistent directory What do you get if you do: $ ls -d /sys/devices/system/cpu/cpu?/online ls: /sys/devices/system/cpu/cpu//online: No such file or directory Somehow, you or your shell replaced the ? character into a / character. You could try with a * instead. With a different RFS your script runs fine without any change. Will update with results once testing is done. The script has been running for a few hours on my 5420 based chromebook. I can see all the cores being hotplugged in/out. A snippet of the prints, that I get on running the test, follows: [...] Good, that looks pretty good. Once you implement full cluster shutdown I can provide you with another script stressing that part. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/5] MCPM backend for Exynos5420
On Wed, 14 May 2014, Abhilash Kesavan wrote: Hi Nicolas, [...] 1) can't create /sys/devices/system/cpu/cpu//online: nonexistent directory What do you get if you do: $ ls -d /sys/devices/system/cpu/cpu?/online ls: /sys/devices/system/cpu/cpu//online: No such file or directory Somehow, you or your shell replaced the ? character into a / character. You could try with a * instead. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/5] MCPM backend for Exynos5420
On Tue, 13 May 2014, Abhilash Kesavan wrote:
This is v6 of the series adding MCPM backend support for SMP secondary boot
and core switching on Samsung's Exynos5420. The patches are based on the mcpm
support added for Exynos5420 in the Chromium kernel repository here:
https://chromium.googlesource.com/chromiumos/third_party/kernel-next/+/chromeos-3.8
The patches have been prepared on Kukjin Kim's for-next branch and tested on
SMDK5420 EVT1 as well as an exynos5420 based chromebook (peach-pit) using the
/dev/b.L_switcher user interface. Secondary core boot-up has also been
tested
on both the boards.
OK... Now it is time for real testing. :-)
The /dev/b.L_switcher interface tests the switcher. Here you really
want to hammer the MCPM functionalities and especially your backend code
as hard as possible. I therefore recommend the following test script:
-- 8
#!/bin/bash
echo 0 /sys/kernel/bL_switcher/active
sleep 1
pids=
for cpu in /sys/devices/system/cpu/cpu?/online; do
{ cpu_nr=${cpu:27:1}
while true; do
echo 1 $cpu 2 /dev/null
sleep .00$RANDOM
val1=$(cat $cpu)
echo 0 $cpu 2 /dev/null
sleep .00$RANDOM
val0=$(cat $cpu)
[ $val1 = 1 -a $val0 = 0 ] echo -n $cpu_nr
done
}
pids=$pids $!
done
trap kill $pids; echo 0 15
wait $pids
-- 8
Leave this running for a couple hours making sure you see all CPU
numbers being printed. The printing order will be random, but each CPU
number should continuously appear.
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 5/5] arm: exynos: Add MCPM call-back functions
On Wed, 14 May 2014, Chander Kashyap wrote:
On 14 May 2014 08:14, Abhilash Kesavan kesavan.abhil...@gmail.com wrote:
Hi Lorenzo,
On Tue, May 13, 2014 at 10:18 PM, Lorenzo Pieralisi
lorenzo.pieral...@arm.com wrote:
On Tue, May 13, 2014 at 12:58:44PM +0100, Abhilash Kesavan wrote:
[...]
+static int __init exynos_mcpm_init(void)
+{
+ struct device_node *node;
+ int ret = 0;
There is no point in initializing it to 0.
OK.
+
+ node = of_find_compatible_node(NULL, NULL, samsung,exynos5420);
+ if (!node)
+ return -ENODEV;
+ of_node_put(node);
+
+ if (!cci_probed())
+ return -ENODEV;
+
+ node = of_find_compatible_node(NULL, NULL,
+ samsung,exynos4210-sysram-ns);
+ if (!node)
+ return -ENODEV;
+
+ ns_sram_base_addr = of_iomap(node, 0);
+ of_node_put(node);
+ if (!ns_sram_base_addr) {
+ pr_err(failed to map non-secure iRAM base address\n);
+ return -ENOMEM;
+ }
+
+ /*
+* To increase the stability of KFC reset we need to program
+* the PMU SPARE3 register
+*/
+ __raw_writel(EXYNOS5420_SWRESET_KFC_SEL, S5P_PMU_SPARE3);
+
+ exynos_mcpm_usage_count_init();
+
+ ret = mcpm_platform_register(exynos_power_ops);
+ if (!ret)
+ ret = mcpm_sync_init(exynos_pm_power_up_setup);
+ if (ret) {
+ iounmap(ns_sram_base_addr);
+ return ret;
+ }
+
+ mcpm_smp_set_ops();
+
+ pr_info(Exynos MCPM support installed\n);
+
+ /*
+* Future entries into the kernel can now go
+* through the cluster entry vectors.
+*/
+ __raw_writel(virt_to_phys(mcpm_entry_point),
+ ns_sram_base_addr + MCPM_BOOT_ADDR_OFFSET);
+
ns_sram_base_addr must be unmapped, since it is unused after the write.
Will unmap.
This mapping is required in for cpuilde (suspend) to program
mcpm_entry before going to suspend.
Why?
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 5/5] arm: exynos: Add MCPM call-back functions
On Wed, 14 May 2014, Chander Kashyap wrote: On 14 May 2014 08:32, Nicolas Pitre nicolas.pi...@linaro.org wrote: On Wed, 14 May 2014, Chander Kashyap wrote: On 14 May 2014 08:14, Abhilash Kesavan kesavan.abhil...@gmail.com wrote: On Tue, May 13, 2014 at 10:18 PM, Lorenzo Pieralisi lorenzo.pieral...@arm.com wrote: On Tue, May 13, 2014 at 12:58:44PM +0100, Abhilash Kesavan wrote: + /* +* Future entries into the kernel can now go +* through the cluster entry vectors. +*/ + __raw_writel(virt_to_phys(mcpm_entry_point), + ns_sram_base_addr + MCPM_BOOT_ADDR_OFFSET); + ns_sram_base_addr must be unmapped, since it is unused after the write. Will unmap. This mapping is required in for cpuilde (suspend) to program mcpm_entry before going to suspend. Why? This is required to program the mcpm_entry point address (resume address) after cpuidle exit/fail. You should be using mcpm_set_entry_vector() for that. Once mcpm_entry_point is set, it shouldn't need to be changed. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/5] MCPM backend for Exynos5420
On Wed, 14 May 2014, Abhilash Kesavan wrote:
Hi Nicolas,
On Tue, May 13, 2014 at 11:25 PM, Nicolas Pitre
nicolas.pi...@linaro.org wrote:
On Tue, 13 May 2014, Abhilash Kesavan wrote:
This is v6 of the series adding MCPM backend support for SMP secondary boot
and core switching on Samsung's Exynos5420. The patches are based on the
mcpm
support added for Exynos5420 in the Chromium kernel repository here:
https://chromium.googlesource.com/chromiumos/third_party/kernel-next/+/chromeos-3.8
The patches have been prepared on Kukjin Kim's for-next branch and tested
on
SMDK5420 EVT1 as well as an exynos5420 based chromebook (peach-pit) using
the
/dev/b.L_switcher user interface. Secondary core boot-up has also been
tested
on both the boards.
OK... Now it is time for real testing. :-)
The /dev/b.L_switcher interface tests the switcher. Here you really
want to hammer the MCPM functionalities and especially your backend code
as hard as possible. I therefore recommend the following test script:
-- 8
#!/bin/bash
echo 0 /sys/kernel/bL_switcher/active
sleep 1
pids=
for cpu in /sys/devices/system/cpu/cpu?/online; do
{ cpu_nr=${cpu:27:1}
while true; do
echo 1 $cpu 2 /dev/null
sleep .00$RANDOM
val1=$(cat $cpu)
echo 0 $cpu 2 /dev/null
sleep .00$RANDOM
val0=$(cat $cpu)
[ $val1 = 1 -a $val0 = 0 ] echo -n $cpu_nr
done
}
pids=$pids $!
done
trap kill $pids; echo 0 15
wait $pids
-- 8
Leave this running for a couple hours making sure you see all CPU
numbers being printed. The printing order will be random, but each CPU
number should continuously appear.
I tried this script and I get two errors:
1) can't create /sys/devices/system/cpu/cpu//online: nonexistent directory
What do you get if you do:
$ ls -d /sys/devices/system/cpu/cpu?/online
?
2) sleep: invalid number '.0026736'
A sufficiently recent coreutils package should have a sleep command that
accepts fractional values. Alternatively you may replace it with
usleep:
usleep $RANDOM
For 1) the cpu number is not being appended. if I give a particular
cpu in the script then hotplug in/out works fine.
For 2) a constant msleep 10 works.
If you have msleep but not usleep then try:
msleep $(($RANDOM / 1000))
Is it OK for me to modify the script to hotplug in/off a randomly chosed core
?
No. They must *all* be hotplugged simultaneously with some random
timing.
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 5/5] arm: exynos: Add MCPM call-back functions
On Mon, 5 May 2014, Abhilash Kesavan wrote:
Add machine-dependent MCPM call-backs for Exynos5420. These are used
to power up/down the secondary CPUs during boot, shutdown, s2r and
switching.
Signed-off-by: Thomas Abraham thomas...@samsung.com
Signed-off-by: Inderpal Singh inderpa...@samsung.com
Signed-off-by: Andrew Bresticker abres...@chromium.org
Signed-off-by: Abhilash Kesavan a.kesa...@samsung.com
Reviewed-by: Nicolas Pitre ni...@linaro.org
---
arch/arm/mach-exynos/Kconfig |8 +
arch/arm/mach-exynos/Makefile |2 +
arch/arm/mach-exynos/mcpm-exynos.c | 344
arch/arm/mach-exynos/regs-pmu.h|3 +
4 files changed, 357 insertions(+)
create mode 100644 arch/arm/mach-exynos/mcpm-exynos.c
diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index 5c34dc2..138070e 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -73,4 +73,12 @@ config SOC_EXYNOS5440
endmenu
+config EXYNOS5420_MCPM
+ bool Exynos5420 Multi-Cluster PM support
+ depends on MCPM SOC_EXYNOS5420
+ select ARM_CCI
+ help
+ This is needed to provide CPU and cluster power management
+ on Exynos5420 implementing big.LITTLE.
+
endif
diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index a656dbe..01bc9b9 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -29,3 +29,5 @@ obj-$(CONFIG_ARCH_EXYNOS) += firmware.o
plus_sec := $(call as-instr,.arch_extension sec,+sec)
AFLAGS_exynos-smc.o :=-Wa,-march=armv7-a$(plus_sec)
+
+obj-$(CONFIG_EXYNOS5420_MCPM)+= mcpm-exynos.o
diff --git a/arch/arm/mach-exynos/mcpm-exynos.c
b/arch/arm/mach-exynos/mcpm-exynos.c
new file mode 100644
index 000..e578007
--- /dev/null
+++ b/arch/arm/mach-exynos/mcpm-exynos.c
@@ -0,0 +1,344 @@
+/*
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * arch/arm/mach-exynos/mcpm-exynos.c
+ *
+ * Based on arch/arm/mach-vexpress/dcscb.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/arm-cci.h
+#include linux/delay.h
+#include linux/io.h
+#include linux/of_address.h
+
+#include asm/cputype.h
+#include asm/cp15.h
+#include asm/mcpm.h
+
+#include regs-pmu.h
+#include common.h
+
+#define EXYNOS5420_CPUS_PER_CLUSTER 4
+#define EXYNOS5420_NR_CLUSTERS 2
+
+/* Non-secure iRAM base address */
+static void __iomem *ns_sram_base_addr;
+
+/*
+ * The common v7_exit_coherency_flush API could not be used because of the
+ * Erratum 799270 workaround. This macro is the same as the common one (in
+ * arch/arm/include/asm/cacheflush.h) except for the erratum handling.
+ */
+#define exynos_v7_exit_coherency_flush(level) \
+ asm volatile( \
+ stmfd sp!, {fp, ip}\n\t\
+ mrcp15, 0, r0, c1, c0, 0 @ get SCTLR\n\t \
+ bicr0, r0, #__stringify(CR_C)\n\t \
+ mcrp15, 0, r0, c1, c0, 0 @ set SCTLR\n\t \
+ isb\n\t\
+ bl v7_flush_dcache___stringify(level)\n\t \
+ clrex\n\t\
+ mrcp15, 0, r0, c1, c0, 1 @ get ACTLR\n\t \
+ bicr0, r0, #(1 6) @ disable local coherency\n\t \
+ /* Dummy Load of a device register to avoid Erratum 799270 */ \
+ ldrr4, [%0]\n\t \
+ andr4, r4, #0\n\t \
+ orrr0, r0, r4\n\t \
+ mcrp15, 0, r0, c1, c0, 1 @ set ACTLR\n\t \
+ isb\n\t \
+ dsb\n\t \
+ ldmfd sp!, {fp, ip} \
+ : \
+ : Ir (S5P_INFORM0) \
+ : r0, r1, r2, r3, r4, r5, r6, r7, \
+ r9, r10, lr, memory)
+
+/*
+ * We can't use regular spinlocks. In the switcher case, it is possible
+ * for an outbound CPU to call power_down() after its inbound counterpart
+ * is already live using the same logical CPU number which trips lockdep
+ * debugging.
+ */
+static arch_spinlock_t exynos_mcpm_lock = __ARCH_SPIN_LOCK_UNLOCKED;
+static int
+cpu_use_count[EXYNOS5420_CPUS_PER_CLUSTER][EXYNOS5420_NR_CLUSTERS];
+
+#define exynos_cluster_usecnt(cluster) \
+ (cpu_use_count[0][cluster] + \
+ cpu_use_count[1][cluster] + \
+ cpu_use_count[2][cluster] + \
+ cpu_use_count[3][cluster])
+
+#define exynos_cluster_unused(cluster) !exynos_cluster_usecnt(cluster)
+
+static int exynos_cluster_power_control(unsigned int cluster, int enable)
+{
+ unsigned int tries = 100;
+ unsigned int val;
+
+ if (enable) {
+ exynos_cluster_powerup(cluster);
+ val = S5P_CORE_LOCAL_PWR_EN;
+ } else {
+ exynos_cluster_powerdown(cluster);
+ val = 0;
+ }
+
+ /* Wait until cluster power control is applied */
+ while (tries
Re: [PATCH] ARM: EXYNOS: cpu hotplug: use v7_exit_coherency_flush macro for cache disabling
On Tue, 22 Apr 2014, Leela Krishna Amudala wrote:
Remove the duplicated code for cache disabling and use
v7_exit_coherency_flush
macro to do the same job.
Signed-off-by: Leela Krishna Amudala leela.kris...@linaro.org
Acked-by: Nicolas Pitre n...@linaro.org
---
cpu hotplug is tested with 3.15-rc1 on Origen(which has cortex A9) and
Arndale octa(which has cortex A7 and A15) boards.
arch/arm/mach-exynos/hotplug.c | 56
++--
1 file changed, 2 insertions(+), 54 deletions(-)
diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
index 5eead53..9eb8d1b 100644
--- a/arch/arm/mach-exynos/hotplug.c
+++ b/arch/arm/mach-exynos/hotplug.c
@@ -24,56 +24,6 @@
#include common.h
#include regs-pmu.h
-static inline void cpu_enter_lowpower_a9(void)
-{
- unsigned int v;
-
- asm volatile(
-mcr p15, 0, %1, c7, c5, 0\n
-mcr p15, 0, %1, c7, c10, 4\n
- /*
- * Turn off coherency
- */
-mrc p15, 0, %0, c1, c0, 1\n
-bic %0, %0, %3\n
-mcr p15, 0, %0, c1, c0, 1\n
-mrc p15, 0, %0, c1, c0, 0\n
-bic %0, %0, %2\n
-mcr p15, 0, %0, c1, c0, 0\n
- : =r (v)
- : r (0), Ir (CR_C), Ir (0x40)
- : cc);
-}
-
-static inline void cpu_enter_lowpower_a15(void)
-{
- unsigned int v;
-
- asm volatile(
-mrc p15, 0, %0, c1, c0, 0\n
-bic %0, %0, %1\n
-mcr p15, 0, %0, c1, c0, 0\n
- : =r (v)
- : Ir (CR_C)
- : cc);
-
- flush_cache_louis();
-
- asm volatile(
- /*
- * Turn off coherency
- */
-mrc p15, 0, %0, c1, c0, 1\n
-bic %0, %0, %1\n
-mcr p15, 0, %0, c1, c0, 1\n
- : =r (v)
- : Ir (0x40)
- : cc);
-
- isb();
- dsb();
-}
-
static inline void cpu_leave_lowpower(void)
{
unsigned int v;
@@ -141,10 +91,8 @@ void __ref exynos_cpu_die(unsigned int cpu)
* appropriate sequence for entering low power.
*/
asm(mrc p15, 0, %0, c0, c0, 0 : =r(primary_part) : : cc);
- if ((primary_part 0xfff0) == 0xc0f0)
- cpu_enter_lowpower_a15();
- else
- cpu_enter_lowpower_a9();
+
+ v7_exit_coherency_flush(louis);
platform_do_lowpower(cpu, spurious);
--
1.7.9.5
--
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/
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 15/17] ARM: exynos: cpuidle: Disable cpuidle for 5440
On Tue, 8 Apr 2014, Daniel Lezcano wrote: On 04/08/2014 02:51 PM, Amit Kucheria wrote: On Tue, Apr 8, 2014 at 5:49 PM, Daniel Lezcano daniel.lezc...@linaro.org mailto:daniel.lezc...@linaro.org wrote: There is no point to register the cpuidle driver for the 5440 as it has only one WFI state which is the default idle function when the cpuidle driver is disabled. By disabling cpuidle we prevent to enter to the governor computation for nothing, thus saving a lot of processing time. The only drawback is the statistic via sysfs on this state which is lost but it is meaningless and it could be retrieved from the ftrace easily. So for the future, you'll only merge platform drivers that enable something more than WFI? Well, I already picked up a driver with only WFI because I knew the next idle states were in preparation. But adding a cpuidle driver just for WFI is no sense and if, for a reason I missed, it is really needed, I guess a generic WFI cpuidle driver for all platforms would be make much more sense. Better relegate WFI-only configurations to arch_cpu_idle(). Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, 27 Jan 2014, Russell King - ARM Linux wrote: On Sun, Jan 26, 2014 at 11:30:00PM -0500, Nicolas Pitre wrote: On Sun, 26 Jan 2014, Russell King - ARM Linux wrote: On Tue, Jan 21, 2014 at 12:45:05AM +, Alan Cox wrote: Peter handed it on. Try using git log on Documentation/devices.txt. It still gets updates. Perhaps you'd care to stick to reality and fix the tree instead of trying to excuse the mess ? Perhaps returning to reality might be advantageous rather than trying to repeat statements which can't have any bearing on this - especially as the git history which you're referring to only goes back to 2.6.12-rc2, and this predates 2.6.12-rc2 by a long shot. More importantly certain folks need to stop abusing static numbers allocated properly. Repeating it having made a total hash of it before is dismal. And if you continue these stupid accusations which have no basis at all, we're going to get into a real big argument, because you are soo _wrong_ on that point. I was always the one arguing /against/ the re-use of existing major/minor numbers. I was the one arguing /against/ Nicolas' patches to make every serial port appear in the 4,64 ttyS namespace. If you remember correctly, that was my attempt at making serial port minor assignment to be dynamic... just like everything else is today. And it seemed to me that you thought this was a good idea. I may have thought that a dynamic space for serial devices was a good idea, but what I was referring to above was specifically the implementation. Oh I do not dispute the fact that the implementation at the time was not up to needed standards. That would have been fixed eventually though, if it hadn't been for the policy based objection we received. At which point this effort was simply dropped. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] serial: samsung: Move uart_register_driver call to device probe
On Sun, 26 Jan 2014, Russell King - ARM Linux wrote: On Tue, Jan 21, 2014 at 12:45:05AM +, Alan Cox wrote: Peter handed it on. Try using git log on Documentation/devices.txt. It still gets updates. Perhaps you'd care to stick to reality and fix the tree instead of trying to excuse the mess ? Perhaps returning to reality might be advantageous rather than trying to repeat statements which can't have any bearing on this - especially as the git history which you're referring to only goes back to 2.6.12-rc2, and this predates 2.6.12-rc2 by a long shot. More importantly certain folks need to stop abusing static numbers allocated properly. Repeating it having made a total hash of it before is dismal. And if you continue these stupid accusations which have no basis at all, we're going to get into a real big argument, because you are soo _wrong_ on that point. I was always the one arguing /against/ the re-use of existing major/minor numbers. I was the one arguing /against/ Nicolas' patches to make every serial port appear in the 4,64 ttyS namespace. If you remember correctly, that was my attempt at making serial port minor assignment to be dynamic... just like everything else is today. And it seemed to me that you thought this was a good idea. But that was objected by the X86 folks who insisted on always having COM1 == ttyS0 and COM2 == ttyS1, whether or not COM1 was present. Nowdays serial ports have pretty much disappeared from the X86 world. But the problem they've created is still with us. Either everything is dynamic, or everything follows proper minor assignment process. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: Add Arm Erratum 773769 for Large data RAM latency.
On Wed, 8 Jan 2014, Doug Anderson wrote: Hi, On Wed, Jan 8, 2014 at 11:20 AM, Russell King - ARM Linux No, we're saying to put the work-around in the boot loader, not the kernel. Unfortunately the resume path of the firmware runs from Read Only firmware code (yes, it sucks), so it's not totally trivial to fix. It would be possible for someone to unscrew their write protect switch and update their RO firmware, but that doesn't help the average user. [...] I'd guess that the way forward is: * Land kernel workaround locally in Chromium tree * I'll try to land FW change locally in at least one Chromium branch. If we were to get a new RO build ever (seems unlikely at this point) it would be fixed for upstream kernels. If we were to get a new RW build (seems unlikely, but at least less unlikely) it would be fixed if someone landed a kernel patch to save/restore this register across suspend/resume. * If Joe Upstream wants to run an upstream kernel on some type of exynos5250 product (Samsung ARM Chromebook, HP Chromebook 11, Nexus 10 are the ones I know of) then he will deal with the small number of crashes or figure out a solution. If Joe Upstream wants to run an upstream kernel, doesn't he have to unscrew his write protect switch first, at which point the RO firmware can be updated as well? Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: Add Arm Erratum 773769 for Large data RAM latency.
On Wed, 8 Jan 2014, Doug Anderson wrote: On Wed, Jan 8, 2014 at 11:20 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: We've been through these arguments many times, you're not the first to raise it, and we've decided upon the policy. We want as _few_ work- arounds in the kernel as possible, because applying the work-arounds is very problematical with the mixture of secure and non-secure booting. OK, fair enough. If we want no workaround here then we can drop this patch. I'd guess that the way forward is: * Land kernel workaround locally in Chromium tree * I'll try to land FW change locally in at least one Chromium branch. If we were to get a new RO build ever (seems unlikely at this point) it would be fixed for upstream kernels. If we were to get a new RW build (seems unlikely, but at least less unlikely) it would be fixed if someone landed a kernel patch to save/restore this register across suspend/resume. * If Joe Upstream wants to run an upstream kernel on some type of exynos5250 product (Samsung ARM Chromebook, HP Chromebook 11, Nexus 10 are the ones I know of) then he will deal with the small number of crashes or figure out a solution. At some point you have to realize that what you're proposing is not viable when taking into account the bigger picture. And your FW architecture is to blame. If you were running Windows instead of Linux I bet you'd have found a way to fix things differently than asking Microsoft to add a special quirk in their code. In the PC world you are required to perform a BIOS update instead. So you really need to provision the ability to update at least a portion of the firmware that is not read-only. That may be a third-stage bootloader or the like which purpose is only to install workarounds such as this before executing the kernel and which doesn't need to be updated with the kernel. And in theory you should be able to do that on top of your current RO firmware. Errata workarounds are often machine specific and they can be applied only in special conditions the kernel might not safely know about until it is too late. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] ARM: EXYNOS: add Exynos Dual Cluster Support
On Tue, 26 Nov 2013, Vyacheslav Tyrtov wrote:
From: Tarek Dakhran t.dakh...@samsung.com
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 t.dakh...@samsung.com
Signed-off-by: Vyacheslav Tyrtov v.tyr...@samsung.com
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-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/4] ARM: EXYNOS: add Exynos Dual Cluster Support
On Thu, 17 Oct 2013, Daniel Lezcano wrote:
On 10/17/2013 04:32 PM, Dave Martin wrote:
On Thu, Oct 17, 2013 at 12:45:29PM +0200, Daniel Lezcano wrote:
On 10/14/2013 05:08 PM, Vyacheslav Tyrtov wrote:
From: Tarek Dakhran t.dakh...@samsung.com
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.
[...]
+ __mcpm_cpu_down(cpu, cluster);
+
+ if (!skip_wfi) {
+ exynos_core_power_down(cpu, cluster);
+ wfi();
+ }
+}
I did not looked line by line but these functions looks very similar
than the tc2_pm.c's function. no ?
This is true.
May be some code consolidation could be considered here.
Added Nico and Lorenzo in Cc.
Thanks
-- Daniel
Nico can commnent further, but I think the main concern here was that
this code shouldn't be factored prematurely.
I do not share this opinion.
There are many low-level platform specifics involved here, so it's
hard to be certain that all platforms could fit into a more abstracted
framework until we have some evidence to look at.
This could be revisited when we have a few diverse MCPM ports to
compare.
I am worried about seeing more and more duplicated code around the ARM arch
(eg. arm[64]/kernel/smp.c arm64/kernel/smp.c).
I didn't look too closely at those files so can't comment much on them.
The cpuidle drivers have been duplicated and it took a while before
refactoring them, and it is not finished. The hotplug code have been
duplicated and now it is very difficult to factor it out.
A lot of work have been done to consolidate the code across the different SoC
since the last 2 years.
If we let duplicate code populate the different files, they will belong to
different maintainers, thus different trees. Each SoC contributors will tend
to add their small changes making the code to diverge more and more and making
difficult to re-factor it later.
I am in favor of preventing duplicate code entering in the kernel and force
the contributors to improve the kernel in general, not just the small part
they are supposed to work on. Otherwise, we are letting the kernel to fork
itself, internally...
Now I'm going to comment.
What you say above is perfectly right. As a general principle, we want
good consolidation. That's the theory.
However you need to know what needs to be consolidated in the first
place. In practice you cannot consolidate duplicated code if that code
doesn't exist. In the cpuidle and hotplug cases it is very easy now to
see what kind of consolidation should have been done after the facts.
I'm sure that 10 years ago that wasn't as obvious.
In the MCPM case we didn't know up front what exactly would end up being
duplicated in each machine specific backend. And to some extent we
still don't know as there is very few backends merged into mainline at
this point (I know more of them exist out of tree but I didn't see the
code yet). Right now we have only 2 such backends and some duplication
was already identified, hence the patch to abstract the v7 cache
flushing I posted recently.
Another possibility for consolidation in the MCPM backends is the last
man determination. However we've seen that the PSCI compatibility
backend doesn't need that and abstracting that just yet might be
premature, possibly making interaction with PSCI very awkward.
So it is very important to get the right balance between code
duplication and over-engineering. Premature abstractions do fall into
the over-engineering category and it is just as bad. Above all it is
most important to be vigilant and take care of duplicated code before it
gets too big.
In the past we've seen bad code duplication in some subsystems because
no one was looking at the big picture. In the MCPM case I'm watching.
And in this case we're far from being in a critical situation.
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] ARM: EXYNOS: add Exynos Dual Cluster Support
On Mon, 7 Oct 2013, Dave Martin wrote: On Fri, Oct 04, 2013 at 03:51:31PM -0400, Nicolas Pitre wrote: No FIQs are supposed to ever race with this code. There is an anomaly though: FIQ and external abort don't seem to get explicitly masked anywhere, either on the suspend or powerdown paths. Sometimes either or both remains unmasked (I tried some trace in the TC2 MCPM backend to confirm this.) Looks like a possible omission in the arch/arm/ suspend and shutdown code, rather than a problem specific to MCPM. Possibly, yes. Feel free to post a patch. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] ARM: EXYNOS: add Exynos Dual Cluster Support
On Wed, 2 Oct 2013, Dave Martin wrote:
On Tue, Oct 01, 2013 at 08:17:04PM +0400, Vyacheslav Tyrtov wrote:
+static int exynos_power_up(unsigned int cpu, unsigned int cluster)
+{
+ int ret;
+ local_irq_disable();
Should there be a local_fiq_disable() here also?
No. In fact this is paired with
+ arch_spin_lock(exynos_lock);
to create the equivalent of a arch_spin_lock_irq(). And the reason is:
/*
* We can't use regular spinlocks. In the switcher case, it is possible
* for an outbound CPU to call power_down() after its inbound counterpart
* is already live using the same logical CPU number which trips lockdep
* debugging.
*/
Otherwise we simply would have used spin_lock_irq().
No FIQs are supposed to ever race with this code.
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] ARM: EXYNOS: add Exynos Dual Cluster Support
On Tue, 1 Oct 2013, Vyacheslav Tyrtov wrote:
From: Tarek Dakhran t.dakh...@samsung.com
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 t.dakh...@samsung.com
Signed-off-by: Vyacheslav Tyrtov v.tyr...@samsung.com
---
arch/arm/mach-exynos/Makefile | 2 +
arch/arm/mach-exynos/edcs.c| 217
+
arch/arm/mach-exynos/edcs.h| 41 +++
arch/arm/mach-exynos/edcs_status.c | 110 +++
4 files changed, 370 insertions(+)
create mode 100644 arch/arm/mach-exynos/edcs.c
create mode 100644 arch/arm/mach-exynos/edcs.h
create mode 100644 arch/arm/mach-exynos/edcs_status.c
diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index 5369615..18e6162 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_MACH_EXYNOS4_DT)+= mach-exynos4-dt.o
obj-$(CONFIG_MACH_EXYNOS5_DT)+= mach-exynos5-dt.o
+
+obj-$(CONFIG_ARM_CCI)+= edcs.o edcs_status.o
diff --git a/arch/arm/mach-exynos/edcs.c b/arch/arm/mach-exynos/edcs.c
new file mode 100644
index 000..34b4e4b
--- /dev/null
+++ b/arch/arm/mach-exynos/edcs.c
@@ -0,0 +1,217 @@
+/*
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ * Author: Tarek Dakhran t.dakh...@samsung.com
+ *
+ * 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.
+ */
+
+#include linux/kernel.h
+#include linux/spinlock.h
+#include linux/errno.h
+#include linux/vexpress.h
Why do you need a vexpress include?
+#include asm/mcpm.h
+#include asm/proc-fns.h
+#include asm/cacheflush.h
+#include asm/cputype.h
+#include asm/cp15.h
+
+#include edcs.h
+
+static arch_spinlock_t exynos_lock = __ARCH_SPIN_LOCK_UNLOCKED;
+static int kfs_use_count[MAX_CPUS_PER_CLUSTER][MAX_NR_CLUSTERS];
The kfs_ prefix looks like a remnant of ancient code which stood for
Kingfisher before it was renamed to DCSCB. :-)
You might consider using edcs_ instead. Same thing for exynos_lock
above which is rather generic a name. Maybe edcs_lock would be better.
Also please define your own constant such as EDCS_CPUS_PER_CLUSTER
instead of using MAX_CPUS_PER_CLUSTER as the later is related to the
code limit and not the actual hardware topology, and it could change in
the future.
+static int core_count[MAX_NR_CLUSTERS]; +
+static int exynos_core_power_control(unsigned int cpu, unsigned int cluster,
+ int enable)
+{
+ unsigned long timeout = jiffies + msecs_to_jiffies(10);
+ unsigned int offset = cluster * MAX_CPUS_PER_CLUSTER + cpu;
Here's an example of where your code would break if MAX_CPUS_PER_CLUSTER
changes.
+ int value = enable ? S5P_CORE_LOCAL_PWR_EN : 0;
+
+ if ((__raw_readl(EXYNOS5410_CORE_STATUS(offset)) 0x3) == value)
+ return 0;
+
+ __raw_writel(value, EXYNOS5410_CORE_CONFIGURATION(offset));
+ do {
+ if ((__raw_readl(EXYNOS5410_CORE_STATUS(offset)) 0x3)
+ == value)
+ return 0;
+ } while (time_before(jiffies, timeout));
+
+ return -EDEADLK;
+}
+static int exynos_cluster_power_control(unsigned int cluster, int enable)
+{
+ unsigned long timeout = jiffies + msecs_to_jiffies(10);
+ int value = enable ? S5P_CORE_LOCAL_PWR_EN : 0;
+
+ if ((__raw_readl(EXYNOS5410_COMMON_STATUS(cluster)) 0x3)
+ == value)
You could put the above if () all on the same line.
+ return 0;
+
+ __raw_writel(value, EXYNOS5410_COMMON_CONFIGURATION(cluster));
+ do {
+ if ((__raw_readl(EXYNOS5410_COMMON_STATUS(cluster))
+ __raw_readl(EXYNOS5410_L2_STATUS(cluster)) 0x3)
+ == value)
+ return 0;
+ } while (time_before(jiffies, timeout));
+
+ return -EDEADLK;
+}
+
+static int exynos_core_power_up(unsigned int cpu, unsigned int cluster)
+{
+ return exynos_core_power_control(cpu, cluster, 1);
+}
+
+static int exynos_core_power_down(unsigned int cpu, unsigned int cluster)
+{
+ return exynos_core_power_control(cpu, cluster, 0);
+}
+
+static int exynos_cluster_power_up(unsigned int cluster)
+{
+ return exynos_cluster_power_control(cluster, 1);
+}
+
+static int exynos_power_up(unsigned int cpu, unsigned int cluster)
+{
+ int ret;
+ local_irq_disable();
+ arch_spin_lock(exynos_lock);
+
+ pr_info(A%d CORE%d\n, 15 - cluster * 8, cpu);
You should consider tuning this down to
Re: [PATCH] ARM: EXYNOS: Fix incorrect usage of S5P_ARM_CORE1_* registers
On Wed, 19 Jun 2013, Tomasz Figa wrote:
On Wednesday 19 of June 2013 20:26:50 Chander Kashyap wrote:
On 19 June 2013 19:58, Tomasz Figa t.f...@samsung.com wrote:
I mean, calculate register offset based on two parameters - cluster ID
and
CPU ID, like:
...
u32 mpidr = cpu_logical_map(cpu);
u32 phys_cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
if (soc_is_exynos()) {
u32 cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
phys_cpu += EXYNOS_CPUS_PER_CLUSTER * cluster;
}
reg_base = S5P_ARM_CORE_CONFIGURATION(phys_cpu);
__raw_writel(0, reg_base);
This does not seems to viable solution, as eg. clusterID for
exynos4210 is 0x9 and exynos 4412 is 0xa.
We don't need to consider cluster ID for any SoC that has just one cluster.
That's why there is the if (soc_is_exynos()) clause, where exynos
is the SoC that we support and has more clusters.
But if we wass the cpu nodes
thru DT, the we can comfortably rely on the logical cpu number. Also
EXYNOS_CPUS_PER_CLUSTER can vary from cluster to cluster.
There is nothing that prevents you from specifying the CPUs in DT in
different order. Moreover, even if you specify them in correct order, there
is nothing that prevents you from using any of the listed CPUs as boot CPU,
which will get the logical ID of 0.
Relying on the logical CPU number to index into hardware related
register space is wrong, please don't do that.
If the MPIDR allocation isn't linear then this cannot be used either.
The best solution is probably to add this reg_base as a property of each
CPU node in DT, and extract it at boot time to stash it into an array
which can be indexed with the logical CPU number afterwards.
Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Early kernel hang with big DTB appended
On Tue, 15 Jan 2013, Tomasz Figa wrote: Hi Nicolas, On Monday 14 of January 2013 17:13:09 Nicolas Pitre wrote: On Fri, 11 Jan 2013, Sascha Hauer wrote: On Thu, Jan 03, 2013 at 04:55:00PM +0100, Tomasz Figa wrote: Hi, I'm observing strange behavior when booting 3.8-rc1 and -rc2 with appended DTB. The kernel hangs very early when the DTB is bigger than some threshold somewhere around 24 KiB. With fullest possible low level UART debugging (and printk patched to use printascii) I'm receiving following output: Uncompressing Linux... done, booting the kernel. Booting Linux on physical CPU 0xa00 Linux version 3.8.0-rc1-00073-gdf6efca-dirty (t.figa@amdc1227) (gcc version 4.5.2 (Gentoo 4.5.2 p1.2, pie-0.4.5) ) #2 SMP PREEMPT Thu Jan 3 15:37:35 CET 2013 CPU: ARMv7 Processor [413fc090] revision 0 (ARMv7), cr=10c53c7d CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache I tested on two Exynos-based boards (exynos4210-trats and one internal exynos4412-based board) and same happens on both. Do you have any ideas? Another thing besides the things already mentioned is that the dtb may not cross a 1MiB boundary. The Kernel uses a single 1Mib section (aligned to 1Mib) to initially map the dtb. Once you cross that boundary parts of the dtb won't be accessible for the Kernel anymore. Crap. You're right. This patch should fix this issue. @Tomasz: please could you confirm this fixes your initial problem? I just tested the patch and it fixes the problem indeed. The kernel now boots successfully after applying the patch, while undoing it makes the kernel fail to boot again. Thanks. Tested-by: Tomasz Figa t.f...@samsung.com Thanks. The patch is queued as 7628/1 in RMK's patch system. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Early kernel hang with big DTB appended
On Fri, 11 Jan 2013, Sascha Hauer wrote: On Thu, Jan 03, 2013 at 04:55:00PM +0100, Tomasz Figa wrote: Hi, I'm observing strange behavior when booting 3.8-rc1 and -rc2 with appended DTB. The kernel hangs very early when the DTB is bigger than some threshold somewhere around 24 KiB. With fullest possible low level UART debugging (and printk patched to use printascii) I'm receiving following output: Uncompressing Linux... done, booting the kernel. Booting Linux on physical CPU 0xa00 Linux version 3.8.0-rc1-00073-gdf6efca-dirty (t.figa@amdc1227) (gcc version 4.5.2 (Gentoo 4.5.2 p1.2, pie-0.4.5) ) #2 SMP PREEMPT Thu Jan 3 15:37:35 CET 2013 CPU: ARMv7 Processor [413fc090] revision 0 (ARMv7), cr=10c53c7d CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache I tested on two Exynos-based boards (exynos4210-trats and one internal exynos4412-based board) and same happens on both. Do you have any ideas? Another thing besides the things already mentioned is that the dtb may not cross a 1MiB boundary. The Kernel uses a single 1Mib section (aligned to 1Mib) to initially map the dtb. Once you cross that boundary parts of the dtb won't be accessible for the Kernel anymore. Crap. You're right. This patch should fix this issue. @Tomasz: please could you confirm this fixes your initial problem? diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S index 4eee351f46..61fcb18c7e 100644 --- a/arch/arm/kernel/head.S +++ b/arch/arm/kernel/head.S @@ -246,6 +246,7 @@ __create_page_tables: /* * Then map boot params address in r2 if specified. +* We map 2 sections in case the ATAGs/DTB crosses a section boundary. */ mov r0, r2, lsr #SECTION_SHIFT movsr0, r0, lsl #SECTION_SHIFT @@ -253,6 +254,8 @@ __create_page_tables: addne r3, r3, #PAGE_OFFSET addne r3, r4, r3, lsr #(SECTION_SHIFT - PMD_ORDER) orrne r6, r7, r0 + strne r6, [r3], #1 PMD_ORDER + addne r6, r6, #1 SECTION_SHIFT strne r6, [r3] #ifdef CONFIG_DEBUG_LL -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Early kernel hang with big DTB appended
On Thu, 3 Jan 2013, Tomasz Figa wrote: Hi, I'm observing strange behavior when booting 3.8-rc1 and -rc2 with appended DTB. The kernel hangs very early when the DTB is bigger than some threshold somewhere around 24 KiB. What is the address where you load your zImage? What if you load it, say, at an offset of 16MB from the start of RAM? Not that this is a fix, but at least that would help isolate the issue. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move CP15 definitions to separate header file
On Fri, 20 Jan 2012, Russell King - ARM Linux wrote:
Avoid namespace conflicts with drivers over the CP15 definitions by
moving CP15 related prototypes and definitions to a private header
file.
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
Acked-by: Nicolas Pitre n...@linaro.org
---
arch/arm/include/asm/cp15.h | 87
++
arch/arm/include/asm/system.h| 77 -
arch/arm/kernel/fiq.c|2 +-
arch/arm/kernel/head-nommu.S |2 +-
arch/arm/kernel/head.S |2 +-
arch/arm/kernel/setup.c |2 +-
arch/arm/mach-exynos/hotplug.c |1 +
arch/arm/mach-realview/hotplug.c |1 +
arch/arm/mach-tegra/hotplug.c|1 +
arch/arm/mach-vexpress/hotplug.c |2 +-
arch/arm/mm/alignment.c |2 +-
arch/arm/mm/cache-feroceon-l2.c |1 +
arch/arm/mm/cache-tauros2.c |1 +
arch/arm/mm/cache-xsc3l2.c |2 +-
arch/arm/mm/ioremap.c|1 +
arch/arm/mm/mmu.c|1 +
arch/arm/mm/pgd.c|1 +
arch/arm/vfp/vfpmodule.c |1 +
18 files changed, 103 insertions(+), 84 deletions(-)
create mode 100644 arch/arm/include/asm/cp15.h
diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
new file mode 100644
index 000..3dabd8dd
--- /dev/null
+++ b/arch/arm/include/asm/cp15.h
@@ -0,0 +1,87 @@
+#ifndef __ASM_ARM_CP15_H
+#define __ASM_ARM_CP15_H
+
+#include asm/system.h
+
+/*
+ * CR1 bits (CP#15 CR1)
+ */
+#define CR_M (1 0)/* MMU enable */
+#define CR_A (1 1)/* Alignment abort enable */
+#define CR_C (1 2)/* Dcache enable*/
+#define CR_W (1 3)/* Write buffer enable */
+#define CR_P (1 4)/* 32-bit exception handler */
+#define CR_D (1 5)/* 32-bit data address range*/
+#define CR_L (1 6)/* Implementation defined */
+#define CR_B (1 7)/* Big endian */
+#define CR_S (1 8)/* System MMU protection*/
+#define CR_R (1 9)/* ROM MMU protection */
+#define CR_F (1 10) /* Implementation defined */
+#define CR_Z (1 11) /* Implementation defined */
+#define CR_I (1 12) /* Icache enable*/
+#define CR_V (1 13) /* Vectors relocated to 0x */
+#define CR_RR(1 14) /* Round Robin cache replacement
*/
+#define CR_L4(1 15) /* LDR pc can set T bit
*/
+#define CR_DT(1 16)
+#define CR_IT(1 18)
+#define CR_ST(1 19)
+#define CR_FI(1 21) /* Fast interrupt (lower latency mode)
*/
+#define CR_U (1 22) /* Unaligned access operation */
+#define CR_XP(1 23) /* Extended page tables
*/
+#define CR_VE(1 24) /* Vectored interrupts
*/
+#define CR_EE(1 25) /* Exception (Big) Endian
*/
+#define CR_TRE (1 28) /* TEX remap enable
*/
+#define CR_AFE (1 29) /* Access flag enable
*/
+#define CR_TE(1 30) /* Thumb exception enable
*/
+
+#ifndef __ASSEMBLY__
+
+#if __LINUX_ARM_ARCH__ = 4
+#define vectors_high() (cr_alignment CR_V)
+#else
+#define vectors_high() (0)
+#endif
+
+extern unsigned long cr_no_alignment;/* defined in entry-armv.S */
+extern unsigned long cr_alignment; /* defined in entry-armv.S */
+
+static inline unsigned int get_cr(void)
+{
+ unsigned int val;
+ asm(mrc p15, 0, %0, c1, c0, 0 @ get CR : =r (val) : : cc);
+ return val;
+}
+
+static inline void set_cr(unsigned int val)
+{
+ asm volatile(mcr p15, 0, %0, c1, c0, 0 @ set CR
+ : : r (val) : cc);
+ isb();
+}
+
+#ifndef CONFIG_SMP
+extern void adjust_cr(unsigned long mask, unsigned long set);
+#endif
+
+#define CPACC_FULL(n)(3 (n * 2))
+#define CPACC_SVC(n) (1 (n * 2))
+#define CPACC_DISABLE(n) (0 (n * 2))
+
+static inline unsigned int get_copro_access(void)
+{
+ unsigned int val;
+ asm(mrc p15, 0, %0, c1, c0, 2 @ get copro access
+ : =r (val) : : cc);
+ return val;
+}
+
+static inline void set_copro_access(unsigned int val)
+{
+ asm volatile(mcr p15, 0, %0, c1, c0, 2 @ set copro access
+ : : r (val) : cc);
+ isb();
+}
+
+#endif
+
+#endif
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index e4c96cc..774c41e 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -15,37 +15,6 @@
#define CPU_ARCH_ARMv7
Re: [GIT PULL] Samsung devel for v3.3
On Tue, 10 Jan 2012, Mark Brown wrote: So, is there anything that people like me who are contributing to rather than maintaining things can do to help here beyond chasing maintainers? Generally my process is roughly to monitor what goes into -next and chase people if things don't make it in there but that's not working well here as things are appearing in -next. Maybe the Samsung maintainer(s) should target early merge into the arm-soc tree instead of going straight to linux-next only. The former ends up in the later anyway. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] Samsung devel for v3.3
On Tue, 10 Jan 2012, Mark Brown wrote: On Tue, Jan 10, 2012 at 01:44:54PM -0500, Nicolas Pitre wrote: On Tue, 10 Jan 2012, Mark Brown wrote: So, is there anything that people like me who are contributing to rather than maintaining things can do to help here beyond chasing maintainers? Maybe the Samsung maintainer(s) should target early merge into the arm-soc tree instead of going straight to linux-next only. The former ends up in the later anyway. That sounds like it'd be helpful overall but it's something that has to be sorted out at the maintainer level. I'm guessing there's not really much that contributors can do here? Maybe if enough contributor pressure builds up something will change... Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-next] make s5p64x0_defconfig build error
On Wed, 14 Dec 2011, Russell King - ARM Linux wrote: On Wed, Dec 14, 2011 at 12:32:44PM +, Will Deacon wrote: On Wed, Dec 14, 2011 at 04:57:10AM +, Axel Lin wrote: I got below build error on linux-next 20111213. CC arch/arm/kernel/process.o In file included from arch/arm/mach-s5p64x0/include/mach/system.h:16, from arch/arm/kernel/process.c:64: arch/arm/plat-samsung/include/plat/system-reset.h:19:2: error: #error Fix me up make[1]: *** [arch/arm/kernel/process.o] Error 1 make: *** [arch/arm/kernel] Error 2 The clue is in the commit message: commit d0f7e2beabe6a116152ccc31959b6654b6ef0071 Author: Russell King rmk+ker...@arm.linux.org.uk Date: Tue Dec 6 12:57:02 2011 + ARM: restart: Temporary #error to persuade platform maintainers to take the restart changes seriously Force builds to fail to ensure that platform maintainers take the restart changes seriously, and sort out fixing their code before the next merge window. Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk Indeed, and note the date, and how long its taken for just one of the platforms to be noticed. It seems to me that no one really cares about whether six of the seven platforms which have the #error build or don't build, so I really think there's a simple solution to this at the next merge window - delete any platform which hasn't _at_ _least_ responded with a proposed patch to this. The platform maintainers have had _enough_ notice of this - by not responding, they're quite simply an obstacle to further consolidation, and we're not going to go through months of waiting for their response time and time again. I've posted the patches to the mailing list, I've copied them asking for help, I've chased them several times, and now they have a #error to deal with. If none of this gets their attention (it seems it doesn't), then frankly their platform is unmaintained, it will be broken at the next merge window, and therefore _should_ be deleted. So... unless things change, we can expect Gemini, almost all Samsung stuff, shmobile, vt8500, and Telechips TCC8k to be at least broken at the next merge window. Let me add to this by pointing to this piece of code in arch/arm/include/asm/pgtable.h: |/* This is a temporary hack until shmobile's DMA area size is sorted out */ |#ifdef CONFIG_ARCH_SHMOBILE |#warning SH-Mobile's consistent DMA size conflicts with VMALLOC_END by 144MB |#undef VMALLOC_END |#define VMALLOC_END 0xF600UL |#endif Again, the reason for this is in the commit message. My hope was that, by the central nature of this header file, the build would be sufficiently overwhelmed with #warning noise that the maintainers would manifest themselves and come forth with the needed fix. After repeated inquiries to the listed maintainers in the MAINTAINERS file, I still didn't receive any feedback at this point, and I still can't find a reason for the current state of things either by looking at the concerned code or the related Git commit logs. So if no one cares about this platform then I propose to delete it for v3.3 as well. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] ARM: Exynos4: Add ioremap interceptor for statically remapped regions
On Thu, 13 Oct 2011, Thomas Abraham wrote: On 12 October 2011 22:00, Thomas Abraham thomas.abra...@linaro.org wrote: On 12 October 2011 21:43, Rob Herring robherri...@gmail.com wrote: On 10/10/2011 03:11 AM, Thomas Abraham wrote: ioremap() request for statically remapped regions are intercepted and the statically assigned virtual address is returned. For requests for which there are no statically remapped regions, the requests are let through. Cc: Kukjin Kim kgene@samsung.com Signed-off-by: Thomas Abraham thomas.abra...@linaro.org --- arch/arm/mach-exynos4/cpu.c | 16 arch/arm/mach-exynos4/include/mach/io.h | 4 2 files changed, 20 insertions(+), 0 deletions(-) You won't need this with Nico's vmalloc.h clean-up series. It does exactly this but generically for all platforms. Ok. Thanks for your suggestion. I will move to using Nico's patches. From Nico's reply to his pull request of vmalloc cleanup series, it looks like that pull request has been withdrawn (hope I am not missing anything here). I'm just postponing it because this depends on a large cleanup in the OMAP code which is being pushed to mainline for the next merge window. Without Nico's series, and gic dt support for exynos4 support requiring this patch, all other workarounds to replace this patch does not seem be correct. So is it acceptable to retain this patch and later rework/drop the exynos4 specific ioremap along with Nico's vmalloc patch series when it is merged. I would guess so. But please CC me on those patches so I know what to look for when rebasing my series. Nicolas
Re: [RFC 2/5] ARM: P2V: avoid initializers and assembly using PHYS_OFFSET
On Tue, 4 Jan 2011, Russell King - ARM Linux wrote: As PHYS_OFFSET will be becoming a variable, we can't have it used in initializers nor assembly code. Replace those in generic code with a run-time initialization. Replace those in platform code using the individual platform specific PLAT_PHYS_OFFSET. Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk Acked-by: Nicolas Pitre nicolas.pi...@linaro.org Same comment as for 1/5. Nicolas -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
