Re: [RFC][PATCH 2/7] ARM: OMAP4: cpuidle - Declare the states with the driver declaration
+ Jean, On Wednesday 21 March 2012 02:57 PM, Daniel Lezcano wrote: > The cpuidle API allows to declare statically the states in the driver > structure. Let's use it. > We do no longer need the fill_cstate function called at runtime and > by the way adding more instructions at boot time. > > Signed-off-by: Daniel Lezcano > --- Jean added the fill_cstate() kind of helpers o.w in the old cpuidle code9OMAP30, static tables were used. Ofcourse those tables were not uinsg the cpuidle driver structure. Regards santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC][PATCH 4/7] ARM: OMAP4: cpuidle - fix static omap4_idle_data declaration
On Wednesday 21 March 2012 02:57 PM, Daniel Lezcano wrote: > Signed-off-by: Daniel Lezcano > --- > arch/arm/mach-omap2/cpuidle44xx.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/mach-omap2/cpuidle44xx.c > b/arch/arm/mach-omap2/cpuidle44xx.c > index 0455858..254f97b 100644 > --- a/arch/arm/mach-omap2/cpuidle44xx.c > +++ b/arch/arm/mach-omap2/cpuidle44xx.c > @@ -33,7 +33,7 @@ struct omap4_idle_statedata { > > #define OMAP4_NUM_STATES 3 > > -struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES]; > +static struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES]; OK Regards santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC][PATCH 0/7] OMAP4 cpuidle cleanup
On Wednesday 21 March 2012 03:21 PM, Daniel Lezcano wrote: > On 03/21/2012 10:36 AM, Shilimkar, Santosh wrote: >> On Wed, Mar 21, 2012 at 2:57 PM, Daniel Lezcano >> wrote: >>> This patchset is a proposition to improve a bit the code. >>> The changes are code cleanup and does not change the behavior of the >>> driver itself. >>> >> Thanks. Will have a look at your series. > > Cool, thanks. > >>> A couple a things call my intention. Why the cpuidle device is set >>> for cpu0 only >> Because the mainline code CPUIDLE is supported along with CPUhotplug. >> This is going to change though with Couple CPUIDLE and corresponding >> OMAP updates. > > Ok, thanks for the information. I will look deeper. What happens to cpu1 > when it is online and has nothing to do ? > >>> and why the WFI is not used ? >>> >> I didn't get this question. Do you mean the generic WFI? > I execute default idle loop. > yes. > >> If yes, then, it's mainly because OMAP need additional >> custom barriers. > > Ah, ok. I am not sure if it is possible but that may be cool if we can > call cpu_do_idle instead with additional barrier. > There is no need. Since code around WFI is customised, it make no sense to call cpu_do_idle(0 ofr only that instruction sake. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC][PATCH 1/7] ARM: OMAP4: cpuidle - Remove unused valid field
On Wednesday 21 March 2012 03:16 PM, Daniel Lezcano wrote: > On 03/21/2012 10:41 AM, Shilimkar, Santosh wrote: >> On Wed, Mar 21, 2012 at 2:57 PM, Daniel Lezcano >> wrote: >>> The 'valid' field is never used in the code, let's remove it. >>> >>> Signed-off-by: Daniel Lezcano >>> --- >> It is used during the registration. This field has been very useful for >> debug when need to disable a C-state etc. >> So unless and until there is a strong reason, i would like to retain it. > > IMO if it used for debug purpose, it should be moved to the debug code > and if the debug code is not upstream, then that 'valid' should not be > here but in the out-of-tree code. > When I said debug, I mean CPUIDLE debug and not any special debug code. > By the way, this may be a debate for nothing because a patchset is on > the way to disable C-states from sysfs. > I see but sysfs won't solve that because you want to disable certain C-state so that CPUIDLE driver don't use that state. Let say if the C4 which is OSWR is broken. In such cases, just setting valid flag let you disable it. Again I don't have strong objection to this change. Regards santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC][PATCH 0/7] OMAP4 cpuidle cleanup
Daniel, On Wednesday 21 March 2012 02:57 PM, Daniel Lezcano wrote: > This patchset is a proposition to improve a bit the code. > The changes are code cleanup and does not change the behavior of the > driver itself. > > A couple a things call my intention. Why the cpuidle device is set for cpu0 > only > and why the WFI is not used ? > > Daniel Lezcano (7): > ARM: OMAP4: cpuidle - Remove unused valid field > ARM: OMAP4: cpuidle - Declare the states with the driver declaration > ARM: OMAP4: cpuidle - Remove the cpuidle_params_table table > ARM: OMAP4: cpuidle - fix static omap4_idle_data declaration > ARM: OMAP4: cpuidle - Initialize omap4_idle_data at compile time > ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly > ARM: OMAP4: cpuidle - remove omap4_idle_data initialization at boot > time > The series looks fine to me in general. This clean-up is applicable for OMAP3 cpuidle code as well. I want Jean to look at this series because some of his earlier clean up has introduced those custom functions which are getting removed in this series. Regards santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 01/17] ARM: proc: add definition of cpu_reset for ARMv6 and ARMv7 cores
Minor nit, On 7/7/2011 8:50 AM, Lorenzo Pieralisi wrote: From: Will Deacon This patch adds simple definitions of cpu_reset for ARMv6 and ARMv7 cores, which disable the MMU via the SCTLR. Signed-off-by: Will Deacon --- arch/arm/mm/proc-v6.S |5 + arch/arm/mm/proc-v7.S |7 +++ 2 files changed, 12 insertions(+), 0 deletions(-) diff --git a/arch/arm/mm/proc-v6.S b/arch/arm/mm/proc-v6.S index 1d2b845..f3b5232 100644 --- a/arch/arm/mm/proc-v6.S +++ b/arch/arm/mm/proc-v6.S @@ -56,6 +56,11 @@ ENTRY(cpu_v6_proc_fin) */ .align 5 ENTRY(cpu_v6_reset) + mrc p15, 0, r1, c1, c0, 0 @ ctrl register + bic r1, r1, #0x1@ ...m + mcr p15, 0, r1, c1, c0, 0 @ disable MMU + mov r1, #0 + mcr p15, 0, r1, c7, c5, 4 @ ISB mov pc, r0 /* diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S index 089c0b5..15d6191 100644 --- a/arch/arm/mm/proc-v7.S +++ b/arch/arm/mm/proc-v7.S @@ -58,9 +58,16 @@ ENDPROC(cpu_v7_proc_fin) *to what would be the reset vector. * *- loc - location to jump to for soft reset + * + * This code must be executed using a flat identity mapping with + * caches disabled. Align the text body. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 02/17] ARM: Add cpu power management notifiers
On 7/7/2011 8:50 AM, Lorenzo Pieralisi wrote: From: Colin Cross During some CPU power modes entered during idle, hotplug and suspend, peripherals located in the CPU power domain, such as the GIC and VFP, may be powered down. Add a notifier chain that allows drivers for those peripherals to be notified before and after they may be reset. Signed-off-by: Colin Cross Tested-by: Kevin Hilman --- arch/arm/Kconfig |7 ++ arch/arm/include/asm/cpu_pm.h | 54 arch/arm/kernel/Makefile |1 + arch/arm/kernel/cpu_pm.c | 181 + 4 files changed, 243 insertions(+), 0 deletions(-) create mode 100644 arch/arm/include/asm/cpu_pm.h create mode 100644 arch/arm/kernel/cpu_pm.c diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 9adc278..356f266 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -183,6 +183,13 @@ config FIQ config ARCH_MTD_XIP bool +config ARCH_USES_CPU_PM + bool + +config CPU_PM + def_bool y + depends on ARCH_USES_CPU_PM&& (PM || CPU_IDLE) + config VECTORS_BASE hex default 0x if MMU || CPU_HIGH_VECTOR diff --git a/arch/arm/include/asm/cpu_pm.h b/arch/arm/include/asm/cpu_pm.h new file mode 100644 index 000..b4bb715 --- /dev/null +++ b/arch/arm/include/asm/cpu_pm.h @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2011 Google, Inc. + * + * Author: + * Colin Cross + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#ifndef _ASMARM_CPU_PM_H +#define _ASMARM_CPU_PM_H + +#include +#include + +/* Event codes passed as unsigned long val to notifier calls */ +enum cpu_pm_event { + /* A single cpu is entering a low power state */ + CPU_PM_ENTER, + + /* A single cpu failed to enter a low power state */ + CPU_PM_ENTER_FAILED, + + /* A single cpu is exiting a low power state */ + CPU_PM_EXIT, + + /* A cpu power domain is entering a low power state */ + CPU_COMPLEX_PM_ENTER, + + /* A cpu power domain failed to enter a low power state */ + CPU_COMPLEX_PM_ENTER_FAILED, + + /* A cpu power domain is exiting a low power state */ + CPU_COMPLEX_PM_EXIT, +}; + +int cpu_pm_register_notifier(struct notifier_block *nb); +int cpu_pm_unregister_notifier(struct notifier_block *nb); + +int cpu_pm_enter(void); +int cpu_pm_exit(void); + +int cpu_complex_pm_enter(void); +int cpu_complex_pm_exit(void); + Can "cpu_complex_pm*" renamed to "cpu_cluster_pm*" as discussed earlier on the list. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 03/17] ARM: gic: Use cpu pm notifiers to save gic state
On 7/7/2011 8:50 AM, Lorenzo Pieralisi wrote: From: Colin Cross When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset. This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers. Signed-off-by: Colin Cross --- arch/arm/common/gic.c | 212 + 1 files changed, 212 insertions(+), 0 deletions(-) diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c index 4ddd0a6..8d62e07 100644 --- a/arch/arm/common/gic.c +++ b/arch/arm/common/gic.c [...] + +static int gic_notifier(struct notifier_block *self, unsigned long cmd, void *v) +{ + int i; + + for (i = 0; i< MAX_GIC_NR; i++) { + switch (cmd) { + case CPU_PM_ENTER: + gic_cpu_save(i); + break; + case CPU_PM_ENTER_FAILED: + case CPU_PM_EXIT: + gic_cpu_restore(i); + break; + case CPU_COMPLEX_PM_ENTER: + gic_dist_save(i); + break; + case CPU_COMPLEX_PM_ENTER_FAILED: + case CPU_COMPLEX_PM_EXIT: + gic_dist_restore(i); + break; + } + } + + return NOTIFY_OK; +} Just to put forth OMAP requirements for GIC and see how much we can leverage these for OMAP. OMAP support GP(general) and HS(secure) devices and implements the trustzone on these devices. on Secure devices the GIC save and restore is completely done by secure ROM code. There are API's for save and restore is automatic on CPU reset based on the last CPU cluster state. On GP devices too, very few GIC registers needs to be saved in a pre-defined memory/register layout and restore is again done by boot-ROM code. OMAP need to enable/disable distributor and CPU interfaces based on CPU power states and that is something can be useful. Would be good if there is a provision to over-write the gic save/restore function using function pointers so that OMAP PM code can use the notifiers. Any more thoughts how we can handle this? We would like to use common ARM code as much as possible. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC PATCH 05/17] ARM: kernel: save/restore kernel IF
On 7/7/2011 8:50 AM, Lorenzo Pieralisi wrote: In order to define a common idle interface for the kernel to enter low power modes, this patch provides include files and code that manages OS calls for low power entry and exit. [] diff --git a/arch/arm/kernel/sr_entry.S b/arch/arm/kernel/sr_entry.S new file mode 100644 index 000..4fa9bef --- /dev/null +++ b/arch/arm/kernel/sr_entry.S @@ -0,0 +1,213 @@ +/* + * Copyright (c) 2008-2011 ARM Ltd + * + * Author(s): Jon Callan, Lorenzo Pieralisi + * + * 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 +#include +#include +#include +#include +#include +#include +#include +#include + + .text + +ENTRY(default_sleep) + b out @ BTAC allocates branch and enters loop mode +idle: @ power down is entered with GIC CPU IF still on which + dsb @ might get wfi instruction to complete before the + wfi @ CPU is shut down -- infinite loop +out: + b idle +ENDPROC(default_sleep) Q: What happens for some reason CPU didn't hit targeted state in IDLE. Does CPU keep looping here forever. On OMAP4, we need to issue additional interconnect barrier before WFI. How can we make provision for the same Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC PATCH 06/17] ARM: kernel: save/restore generic infrastructure
On 7/7/2011 8:50 AM, Lorenzo Pieralisi wrote: This patch provides the code infrastructure needed to maintain a generic per-cpu architecture implementation of idle code. sr_platform.c : - code manages patchset initialization and memory management sr_context.c: - code initializes run-time context save/restore generic support sr_power.c: - provides the generic infrastructure to enter exit low power modes and communicate with Power Control Unit (PCU) v7 support hinges on the basic infrastructure to provide per-cpu arch implementation basically through standard function pointers signatures. Preprocessor defines include size of data needed to save/restore L2 state. This define value should be moved to the respective subsystem (PL310) once the patchset IF to that subsystem is settled. Signed-off-by: Lorenzo Pieralisi --- [...] diff --git a/arch/arm/kernel/sr_helpers.h b/arch/arm/kernel/sr_helpers.h new file mode 100644 index 000..1ae3a9a --- /dev/null +++ b/arch/arm/kernel/sr_helpers.h @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2008-2011 ARM Limited + * Author(s): Jon Callan, Lorenzo Pieralisi + * + * 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. + * + */ + +static inline int sr_platform_get_cpu_index(void) +{ + unsigned int cpu; + __asm__ __volatile__( + "mrc p15, 0, %0, c0, c0, 5\n\t" + : "=r" (cpu)); + return cpu& 0xf; +} + +/* + * Placeholder for further extensions + */ +static inline int sr_platform_get_cluster_index(void) +{ + return 0; +} + +static inline void __iomem *sr_platform_cbar(void) +{ + void __iomem *base; + __asm__ __volatile__( + "mrc p15, 4, %0, c15, c0, 0\n\t" + : "=r" (base)); + return base; +} + +#ifdef CONFIG_SMP +static inline void exit_coherency(void) +{ + unsigned int v; + asm volatile ( + "mrc p15, 0, %0, c1, c0, 1\n" + "bic %0, %0, %1\n" + "mcr p15, 0, %0, c1, c0, 1\n" You should have a isb here. +: "=&r" (v) +: "Ir" (0x40) +: ); +} To avoid aborts on platform which doesn't provide access to SMP bit, NSACR bit 18 should be read. Something like mrc p15, 0, r0, c1, c1, 2 tst r0, #(1 << 18) mrcne p15, 0, r0, c1, c0, 1 bicne r0, r0, #(1 << 6) mcrne p15, 0, r0, c1, c0, 1 Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 03/17] ARM: gic: Use cpu pm notifiers to save gic state
On 7/7/2011 6:41 PM, Colin Cross wrote: On Thu, Jul 7, 2011 at 6:35 PM, Santosh Shilimkar wrote: On 7/7/2011 8:50 AM, Lorenzo Pieralisi wrote: From: Colin Cross When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset. This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers. Signed-off-by: Colin Cross --- arch/arm/common/gic.c | 212 + 1 files changed, 212 insertions(+), 0 deletions(-) diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c index 4ddd0a6..8d62e07 100644 --- a/arch/arm/common/gic.c +++ b/arch/arm/common/gic.c [...] + +static int gic_notifier(struct notifier_block *self, unsigned long cmd, void *v) +{ + int i; + + for (i = 0; i<MAX_GIC_NR; i++) { + switch (cmd) { + case CPU_PM_ENTER: + gic_cpu_save(i); + break; + case CPU_PM_ENTER_FAILED: + case CPU_PM_EXIT: + gic_cpu_restore(i); + break; + case CPU_COMPLEX_PM_ENTER: + gic_dist_save(i); + break; + case CPU_COMPLEX_PM_ENTER_FAILED: + case CPU_COMPLEX_PM_EXIT: + gic_dist_restore(i); + break; + } + } + + return NOTIFY_OK; +} Just to put forth OMAP requirements for GIC and see how much we can leverage these for OMAP. OMAP support GP(general) and HS(secure) devices and implements the trustzone on these devices. on Secure devices the GIC save and restore is completely done by secure ROM code. There are API's for save and restore is automatic on CPU reset based on the last CPU cluster state. On GP devices too, very few GIC registers needs to be saved in a pre-defined memory/register layout and restore is again done by boot-ROM code. OMAP need to enable/disable distributor and CPU interfaces based on CPU power states and that is something can be useful. Would be good if there is a provision to over-write the gic save/restore function using function pointers so that OMAP PM code can use the notifiers. Any more thoughts how we can handle this? We would like to use common ARM code as much as possible. Is it strictly necessary to use the custom OMAP save and restore? Yes. On secure devices there is no choice. Anything that was modified by the kernel is obviously writable, and could be saved and restored using the common code. Anything that can only be modified by TrustZone already has to be restored by the custom OMAP code. There aren't many registers in the GIC, so it shouldn't be much of a performance difference. In that case we will end up doing things two time un-necessary and that's not useful at all. You need to save all those extra cycles to have less latency on C-states. And otherside you can't skip the secure API save otherwise the boot-ROM code will end up re-initializing the GIC and all secure interrupt state will be lost. From above code, today we use need dist/cpu interface disable/enable functions. Regards Santosh Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC PATCH 12/17] ARM: kernel: add SCU reset hook
On 7/7/2011 8:50 AM, Lorenzo Pieralisi wrote: When a CLUSTER is powered down the SCU must be reinitialized on warm-boot. This patch adds a hook to reset the SCU, which implies invalidating TAG RAMs and renabling it. The scu virtual address is saved in a static variable when the SCU is first enabled at boot; this allows common idle code to be generic and avoid relying on platform code to get the address at run-time. On warm-boot the SCU TAG RAM is invalidated and the SCU enabled if it is not already enabled. The reset can be skipped altogether thanks to save/restore framework flags. Flushing D$ cache is cumbersome since the system just comes out of reset, which invalidates caches in the process if needed (A9), that is why the scu_enable function is not reused as it is to reset the SCU. If the init function is extended, there might not be a need for a SCU specific hook, since the init function can be reused to reinitialize the SCU at boot provided it is removed from the init section and kept in memory. Signed-off-by: Lorenzo Pieralisi --- arch/arm/include/asm/smp_scu.h |3 ++- arch/arm/kernel/smp_scu.c | 33 ++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h index 4eb6d00..cfaa68e 100644 --- a/arch/arm/include/asm/smp_scu.h +++ b/arch/arm/include/asm/smp_scu.h @@ -8,7 +8,8 @@ #ifndef __ASSEMBLER__ unsigned int scu_get_core_count(void __iomem *); void scu_enable(void __iomem *); -int scu_power_mode(void __iomem *, unsigned int); +int scu_power_mode(unsigned int); +void scu_reset(void); #endif #endif diff --git a/arch/arm/kernel/smp_scu.c b/arch/arm/kernel/smp_scu.c index a1e757c..980ced9 100644 --- a/arch/arm/kernel/smp_scu.c +++ b/arch/arm/kernel/smp_scu.c @@ -20,6 +20,7 @@ #define SCU_INVALIDATE0x0c #define SCU_FPGA_REVISION 0x10 +static void __iomem *scu_va; Change log and patch doesn't seems to match. I remember suggesting this change to Russell when "scu_power_mode()" was introduced. His preference was to have scu_base passed as part of the API. /* * Get the number of CPU cores from the SCU configuration */ @@ -36,6 +37,7 @@ void __init scu_enable(void __iomem *scu_base) { u32 scu_ctrl; + scu_va = scu_base; scu_ctrl = __raw_readl(scu_base + SCU_CTRL); /* already enabled? */ if (scu_ctrl& 1) @@ -59,7 +61,7 @@ void __init scu_enable(void __iomem *scu_base) * has the side effect of disabling coherency, caches must have been * flushed. Interrupts must also have been disabled. */ -int scu_power_mode(void __iomem *scu_base, unsigned int mode) +int scu_power_mode(unsigned int mode) { unsigned int val; int cpu = smp_processor_id(); @@ -67,9 +69,34 @@ int scu_power_mode(void __iomem *scu_base, unsigned int mode) if (mode> 3 || mode == 1 || cpu> 3) return -EINVAL; - val = __raw_readb(scu_base + SCU_CPU_STATUS + cpu)& ~0x03; + val = __raw_readb(scu_va + SCU_CPU_STATUS + cpu)& ~0x03; val |= mode; - __raw_writeb(val, scu_base + SCU_CPU_STATUS + cpu); + __raw_writeb(val, scu_va + SCU_CPU_STATUS + cpu); return 0; } + +/* + * Reinitialise the SCU after power-down + */ + +void scu_reset(void) +{ + u32 scu_ctrl; + + scu_ctrl = __raw_readl(scu_va + SCU_CTRL); + /* already enabled? */ + if (scu_ctrl& 1) + return; + /* +* SCU TAGS should be invalidated on boot-up +*/ + __raw_writel(0x, scu_va + SCU_INVALIDATE); + /* +* Coming out of reset, dcache invalidated +* no need to go through the whole hog again +* just enable the SCU and pop out +*/ + scu_ctrl |= 1; + __raw_writel(scu_ctrl, scu_va + SCU_CTRL); +} ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC PATCH 13/17] ARM: mm: L2x0 save/restore support
On 7/7/2011 8:50 AM, Lorenzo Pieralisi wrote: When the system hits deep low power states the L2 cache controller can lose its internal logic values and possibly its TAG/DATA RAM content. This patch adds save/restore hooks to the L2x0 subsystem to save/restore L2x0 registers and clean/invalidate/disable the cache controller as needed. The cache controller has to go to power down disabled even if its RAM(s) are retained to prevent it from sending AXI transactions on the bus when the cluster is shut-down which might leave the system in a limbo state. Hence the save function cleans (completely or partially) L2 and disable it in one single function to avoid playing with cacheable stack and flush data to L3. The current code saving context for retention mode is still a hack and must be improved. Fully tested on dual-core A9 cluster. Signed-off-by: Lorenzo Pieralisi --- arch/arm/include/asm/outercache.h | 22 + arch/arm/mm/cache-l2x0.c | 63 + 2 files changed, 85 insertions(+), 0 deletions(-) diff --git a/arch/arm/include/asm/outercache.h b/arch/arm/include/asm/outercache.h index d838743..0437c21 100644 --- a/arch/arm/include/asm/outercache.h +++ b/arch/arm/include/asm/outercache.h @@ -34,6 +34,8 @@ struct outer_cache_fns { void (*sync)(void); #endif void (*set_debug)(unsigned long); + void (*save_context)(void *, bool, unsigned long); + void (*restore_context)(void *, bool); }; #ifdef CONFIG_OUTER_CACHE @@ -74,6 +76,19 @@ static inline void outer_disable(void) outer_cache.disable(); } +static inline void outer_save_context(void *data, bool dormant, + phys_addr_t end) +{ + if (outer_cache.save_context) + outer_cache.save_context(data, dormant, end); +} + +static inline void outer_restore_context(void *data, bool dormant) +{ + if (outer_cache.restore_context) + outer_cache.restore_context(data, dormant); +} + #else static inline void outer_inv_range(phys_addr_t start, phys_addr_t end) @@ -86,6 +101,13 @@ static inline void outer_flush_all(void) { } static inline void outer_inv_all(void) { } static inline void outer_disable(void) { } +static inline void outer_save_context(void *data, bool dormant, + phys_addr_t end) +{ } + +static inline void outer_restore_context(void *data, bool dormant) +{ } + #endif #ifdef CONFIG_OUTER_CACHE_SYNC diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c index ef59099..331fe9b 100644 --- a/arch/arm/mm/cache-l2x0.c +++ b/arch/arm/mm/cache-l2x0.c @@ -270,6 +270,67 @@ static void l2x0_disable(void) spin_unlock_irqrestore(&l2x0_lock, flags); } +static void l2x0_save_context(void *data, bool dormant, unsigned long end) +{ + u32 *l2x0_regs = (u32 *) data; + *l2x0_regs = readl_relaxed(l2x0_base + L2X0_AUX_CTRL); + l2x0_regs++; + *l2x0_regs = readl_relaxed(l2x0_base + L2X0_TAG_LATENCY_CTRL); + l2x0_regs++; + *l2x0_regs = readl_relaxed(l2x0_base + L2X0_DATA_LATENCY_CTRL); + + if (!dormant) { + /* clean entire L2 before disabling it*/ + writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_CLEAN_WAY); + cache_wait_way(l2x0_base + L2X0_CLEAN_WAY, l2x0_way_mask); + } else { + /* +* This is an ugly hack, which is there to clean +* the stack from L2 before disabling it +* The only alternative consists in using a non-cacheable stack +* but it is poor in terms of performance since it is only +* needed for cluster shutdown and L2 retention +* On L2 off mode the cache is cleaned anyway +*/ + register unsigned long start asm("sp"); + start&= ~(CACHE_LINE_SIZE - 1); + while (start< end) { + cache_wait(l2x0_base + L2X0_CLEAN_LINE_PA, 1); + writel_relaxed(__pa(start), l2x0_base + + L2X0_CLEAN_LINE_PA); + start += CACHE_LINE_SIZE; + } + } I think you need a cache_sync() here. + /* +* disable the cache implicitly syncs +*/ + writel_relaxed(0, l2x0_base + L2X0_CTRL); +} + +static void l2x0_restore_context(void *data, bool dormant) +{ + u32 *l2x0_regs = (u32 *) data; + + if (!(readl_relaxed(l2x0_base + L2X0_CTRL)& 1)) { + + writel_relaxed(*l2x0_regs, l2x0_base + L2X0_AUX_CTRL); + l2x0_regs++; + writel_relaxed(*l2x0_regs, l2x0_base + L2X0_TAG_LATENCY_CTRL); + l2x0_regs++; + writel_relaxed(*l2x0_regs, l2x0_base + L2X0_DATA_LATENCY_CTRL); + /* +* If L2 is retained do not invalidate +*/ + if
Re: [RFC PATCH 14/17] ARM: kernel: save/restore 1:1 page tables
On 7/7/2011 8:50 AM, Lorenzo Pieralisi wrote: This patch adds the code required to allocate and populate page tables that are needed by save/restore code to deal with MMU off/on transactions. MMU is enabled early in the resume path which allows to call into Linux subsystems with init_mm virtual mappings (cloned at boot). Current thread page table pointer and context id is saved on power down from active_mm and restored on warm boot. Currently the translation tables contains 1:1 mappings of the Linux kernel code and data, and 1:1 UNCACHED mapping of control code required when MMU is turned off in the restore code path. Signed-off-by: Lorenzo Pieralisi --- arch/arm/kernel/sr_mapping.c | 78 ++ 1 files changed, 78 insertions(+), 0 deletions(-) create mode 100644 arch/arm/kernel/sr_mapping.c diff --git a/arch/arm/kernel/sr_mapping.c b/arch/arm/kernel/sr_mapping.c new file mode 100644 index 000..32640dc --- /dev/null +++ b/arch/arm/kernel/sr_mapping.c @@ -0,0 +1,78 @@ +/* + * Copyright (C) 2008-2011 ARM Limited This is more of question so don't beat me if I am wrong here. Above file doesn't exist in k.org from 2008 right ? I noticed this in your other patches too. Regards santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC PATCH 17/17] ARM: kernel: save/restore build infrastructure
On 7/7/2011 8:50 AM, Lorenzo Pieralisi wrote: This patch adds the required Kconfig and Makefile entries to enable and compile common idle code for ARM kernel. Common idle code depends on CPU_PM platform notifiers to trigger save/restore of kernel subsystems like PMU, VFP, GIC. Signed-off-by: Lorenzo Pieralisi --- arch/arm/Kconfig | 11 +++ arch/arm/kernel/Makefile |4 2 files changed, 15 insertions(+), 0 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 356f266..5b670bd 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1992,6 +1992,17 @@ config VFP Say N if your target does not have VFP hardware. +config CONTEXT_SR SR sounds cryptic. Also since it's PM related _PM_ would be good. + bool "Save/Restore code support for CPU/Cluster Power Management" + depends on CPU_V7&& CPU_PM ^ space needed. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 03/17] ARM: gic: Use cpu pm notifiers to save gic state
On 7/9/2011 4:05 PM, Russell King - ARM Linux wrote: On Sat, Jul 09, 2011 at 04:01:19PM -0700, Colin Cross wrote: On Sat, Jul 9, 2011 at 3:33 PM, Russell King - ARM Linux wrote: On Sat, Jul 09, 2011 at 03:10:56PM -0700, Colin Cross wrote: This is necessary for cpuidle states that lose the GIC registers, not just suspend, because the GIC is in the cpu's power domain. We could avoid saving and restoring all the GIC registers in suspend and idle by reusing the initialization functions, and then having the core irq code call the unmask, set_type, and set_affinity functions on each irq to reconfigure it, but that will be very inefficient - it will convert each register write in the restore functions to a read-modify-write per interrupt in that register. Santosh is already complaining that this commong GIC restore code will be slower than the automatic DMA to restore the GIC registers that OMAP4 supports. Well, we need to come up with something sensible - a way of doing this which doesn't require every interrupt controller driver (of which we as an architecture have many) to have lots of support added. If the current way is inefficient and is noticably so, then let's talk to Thomas about finding a way around that - maybe having the generic code make one suspend/resume callback per irq gc chip rather than doing it per-IRQ. We can then reuse the same paths for suspend/resume as for idle state saving. Are you referring to moving the gic driver to be gc chip? Otherwise, I don't understand your suggestion - how is callback per chip any different than what this patch implements? It just gets it's notification through a cpu_pm notifier, which works in idle and suspend, instead of a syscore op like the gc driver does. This patch does save and restore some registers that are never modified after init, so they don't need to be saved. The point is that we should aim to get to the point where, if an interrupt controller supports PM, then it supports _all_ PM out the box and doesn't require additional code for cpu idle PM vs system suspend PM. In other words, all we should need to do is provide genirq with a couple of functions for 'save state' and 'restore state'. Agreed. But how generic irq code will know about when the interrupt controller looses it's context without notifiers. This still depend on the SOC power domain partitions and hence platform code At least for the GIC case, it seems depends on the CPU cluster low power state. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC PATCH 05/17] ARM: kernel: save/restore kernel IF
(Just to add few more points on top of what Colin already commented) On 7/11/2011 11:40 AM, Russell King - ARM Linux wrote: On Mon, Jul 11, 2011 at 03:00:47PM +0100, Lorenzo Pieralisi wrote: Well, short answer is no. On SMP we do need to save CPU registers but if just one single cpu is shutdown L2 is still on. cpu_suspend saves regs on the stack that has to be cleaned from L2 before shutting a CPU down which make things more complicated than they should. Hang on. Please explain something to me here. You've mentioned a few times that cpu_suspend() can't be used because of the L2 cache. Why is this the case? OMAP appears to have code in its sleep path - which has been converted to cpu_suspend() support - to deal with the L2 issues. This part is not done yet Russell. Infact the cpu_resume() function need an update to work with L2 enable configuration. However, lets recap. What we do in cpu_suspend() in order is: - Save the ABI registers onto the stack, and some private state - Save the CPU specific state onto the stack We need to disable C bit here to avoid any speculative artifacts during further operations before WFI. - Flush the L1 cache - Call the platform specific suspend finisher Also finisher function should issue the WFI and just in case for some reason CPU doesn't hit the targeted low power state, finisher function takes care of things like enabling C bit, SMP bit etc. On resume, with the MMU and caches off: - Platform defined entry point is called, which _may_ be cpu_resume directly. - Platform initial code is executed to do whatever that requires - cpu_resume will be called - cpu_resume loads the previously saved private state - The CPU specific state is restored - Page table is modified to permit 1:1 mapping for MMU enable 1:1 idmap used here should be mapped as non-cached to avoid L2 related issues. I faced similar issue in OMAP sleep code earlier and later thanks to Colin, we got the fixed my making use of non-cached idmap. - MMU is enabled with caches disabled - Page table modification is undone - Caches are enabled in the main control register - CPU exception mode stacks are reinitialized - CPU specific init function is called - ABI registers are popped off and 'cpu_suspend' function returns So, as far as L2 goes, in the suspend finisher: - If L2 state is lost, the finisher needs to clean dirty data from L2 to ensure that it is preserved in RAM. Note: There is no need to disable or even invalidate the L2 cache as we should not be writing any data in the finisher function which we later need after resume. - If L2 state is not lost, the finisher needs to clean the saved state as a minimum, to sure that this is visible when the main control register C bit is clear. The easiest way to do that is to find the top of stack via current_thread_info() - we have a macro for that, and then add THREAD_SIZE to find the top of stack. 'sp' will be the current bottom of stack. In the resume initial code: - If L2 state was lost, the L2 configuration needs to be restored. This generally needs to happen before cpu_resume is called: - there are CPUs which need L2 setup before the MMU is enabled. - OMAP3 currently does this in its assembly, which is convenient to allow it to make the SMI calls to the secure world. The same will be true of any CPU running in non-secure mode. This is indeed good approach. It does help to handle the platform specific requirements like trustzone, secure restore/overwrite etc. - If L2 state was not lost, and the platform choses not to clean and invalidate the ABI registers from the stack, and the platform restores the L2 configuration before calling cpu_resume, then the ABI registers will be read out of L2 on return if that's where they are - at that point everything will be setup correctly. This will give the greatest performance, which is important for CPU idle use of these code paths. Now, considering SMP, there's an issue here: do we know at the point where one CPU goes down whether L2 state will be lost? If the answer is that state will not be lost, we can do the minimum. If all L2 state is lost, we need to do as above. If we don't know the answer, then we have to assume that L2 state will be lost. But wait - L2 cache (or more accurately, the outer cache) is common between CPUs in a SMP system. So, if we're _not_ the last CPU to go down, then we assume that L2 state will not be lost. It is the last CPUs responsibility to deal with L2 state when it goes into a PM mode that results in L2 state being lost. This is exactly the point. Unless and until CPU cluster is going down, L2 cache won't be lost. At least that's how OMAP is implemented. Lastly, should generic code deal with flushing L2 and setting it back up on resume? A couple of points there: 1. Will generic code know whether L2 state will be lost, or should it assume that L2 state is always lost and do a ful
Re: [RFC PATCH 05/17] ARM: kernel: save/restore kernel IF
On 7/11/2011 12:19 PM, Russell King - ARM Linux wrote: On Mon, Jul 11, 2011 at 11:51:00AM -0700, Colin Cross wrote: On Mon, Jul 11, 2011 at 11:40 AM, Russell King - ARM Linux wrote: On Mon, Jul 11, 2011 at 03:00:47PM +0100, Lorenzo Pieralisi wrote: Well, short answer is no. On SMP we do need to save CPU registers but if just one single cpu is shutdown L2 is still on. cpu_suspend saves regs on the stack that has to be cleaned from L2 before shutting a CPU down which make things more complicated than they should. Hang on. Please explain something to me here. You've mentioned a few times that cpu_suspend() can't be used because of the L2 cache. Why is this the case? OMAP appears to have code in its sleep path - which has been converted to cpu_suspend() support - to deal with the L2 issues. OMAP is very different, because it doesn't use cpu_suspend. It saves it's state to SAR ram, which is mapped uncached, which avoids L2 problems. I'm afraid your information is out of date. See: I think the confusion is OMAP3 and OMAP4. Colin was talking about OMAP4 which isn't merged in mainline yet where as you were referring OMAP3 clean-ups happened recently. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC PATCH 05/17] ARM: kernel: save/restore kernel IF
On 7/11/2011 1:14 PM, Russell King - ARM Linux wrote: On Mon, Jul 11, 2011 at 01:05:20PM -0700, Santosh Shilimkar wrote: (Just to add few more points on top of what Colin already commented) On 7/11/2011 11:40 AM, Russell King - ARM Linux wrote: On Mon, Jul 11, 2011 at 03:00:47PM +0100, Lorenzo Pieralisi wrote: Well, short answer is no. On SMP we do need to save CPU registers but if just one single cpu is shutdown L2 is still on. cpu_suspend saves regs on the stack that has to be cleaned from L2 before shutting a CPU down which make things more complicated than they should. Hang on. Please explain something to me here. You've mentioned a few times that cpu_suspend() can't be used because of the L2 cache. Why is this the case? OMAP appears to have code in its sleep path - which has been converted to cpu_suspend() support - to deal with the L2 issues. This part is not done yet Russell. Infact the cpu_resume() function need an update to work with L2 enable configuration. The code does deal with L2 cache enable in the resume path... However, lets recap. What we do in cpu_suspend() in order is: - Save the ABI registers onto the stack, and some private state - Save the CPU specific state onto the stack We need to disable C bit here to avoid any speculative artifacts during further operations before WFI. Which you are doing. - Flush the L1 cache - Call the platform specific suspend finisher Also finisher function should issue the WFI and just in case for some reason CPU doesn't hit the targeted low power state, finisher function takes care of things like enabling C bit, SMP bit etc. You're restoring the C bit in the non-off paths already which follow a failed WFI. You're not touching the SMP bit there though - was it ever reset? I don't think so. Probably it's not in the code what you have seen but it's being used in the code base. One tricky issue there is SMP bit access is protected on OMAP4430 GP silicon where as it is opened up on OMAP4460. We handle that using NSACR register read and that's what I pointed to Lorenzo. On resume, with the MMU and caches off: - Platform defined entry point is called, which _may_ be cpu_resume directly. - Platform initial code is executed to do whatever that requires - cpu_resume will be called - cpu_resume loads the previously saved private state - The CPU specific state is restored - Page table is modified to permit 1:1 mapping for MMU enable 1:1 idmap used here should be mapped as non-cached to avoid L2 related issues. I faced similar issue in OMAP sleep code earlier and later thanks to Colin, we got the fixed my making use of non-cached idmap. It is specified that if the main control register C bit is clear, accesses will be uncached. Does it apply for page table walks as well because that's the case which fails. Whether you get cache hits or not is probably implementation dependent, and provided that the state information is cleaned from the L2 cache, it doesn't matter if we hit the L2 cached copy or the RAM copy. It's the same data. I am not sure. Because restored TTRB0 still tells processor that the entry is cached and then CPU instead of reading the entry from main memory(written before MMU OFF) it reads a stale entry from L2 which is the problem. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH] ARM: do not mark CPU 0 as hotpluggable
On 7/21/2011 8:32 AM, Rob Herring wrote: On 07/20/2011 06:32 PM, Mike Turquette wrote: A quick poll of the ARM platforms that implement CPU Hotplug support shows that every platform treats CPU 0 as a special case that cannot be hotplugged. In fact every platform has identical code for platform_cpu_die which returns -EPERM in the case of CPU 0. The user-facing sysfs interfaces should reflect this by not populating an 'online' entry for CPU 0 at all. This better reflects reality by making it clear to users that CPU 0 cannot be hotplugged. This patch prevents CPU 0 from being marked as hotpluggable on all ARM platforms during CPU registration. This in turn prevents the creation of an 'online' sysfs interface for that CPU. Unless there is a kernel limitation why CPU0 can't be hot unplugged, then this should remain a platform decision. This may be another case of everybody just copying other platforms' code, not a platform limitation. Just talking on behalf of OMAP, we can't offline CPU0 and limitation would remain in future OMAPs too. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 03/17] ARM: gic: Use cpu pm notifiers to save gic state
Lorenzo, Colin, On 7/7/2011 9:20 PM, Lorenzo Pieralisi wrote: From: Colin Cross When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset. This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers. Signed-off-by: Colin Cross --- arch/arm/common/gic.c | 212 + 1 files changed, 212 insertions(+), 0 deletions(-) diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c index 4ddd0a6..8d62e07 100644 --- a/arch/arm/common/gic.c +++ b/arch/arm/common/gic.c [...] I missed one more comment in the last review. +static int gic_notifier(struct notifier_block *self, unsigned long cmd, void *v) +{ + int i; + + for (i = 0; i< MAX_GIC_NR; i++) { + switch (cmd) { + case CPU_PM_ENTER: + gic_cpu_save(i); On OMAP, GIC cpu interface context is lost only when CPU cluster is powered down. + break; + case CPU_PM_ENTER_FAILED: + case CPU_PM_EXIT: + gic_cpu_restore(i); + break; + case CPU_COMPLEX_PM_ENTER: + gic_dist_save(i); + break; + case CPU_COMPLEX_PM_ENTER_FAILED: + case CPU_COMPLEX_PM_EXIT: + gic_dist_restore(i); + break; + } + } + + return NOTIFY_OK; +} + Entire GIC is kept in CPU cluster power domain and hence GIC CPU interface context won't be lost whenever CPU alone enters in the deepest power state. If it is different on other SOC's then the common notifiers won't match to all the SOC designs. Looks like exporting these functions directly or adding them to gen irq and then invoking them from platform code based on power sequence need might be better. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 03/17] ARM: gic: Use cpu pm notifiers to save gic state
On 7/21/2011 3:57 PM, Lorenzo Pieralisi wrote: On Thu, Jul 21, 2011 at 09:32:12AM +0100, Santosh Shilimkar wrote: Lorenzo, Colin, On 7/7/2011 9:20 PM, Lorenzo Pieralisi wrote: From: Colin Cross When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset. This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers. Signed-off-by: Colin Cross --- arch/arm/common/gic.c | 212 + 1 files changed, 212 insertions(+), 0 deletions(-) diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c index 4ddd0a6..8d62e07 100644 --- a/arch/arm/common/gic.c +++ b/arch/arm/common/gic.c [...] I missed one more comment in the last review. +static int gic_notifier(struct notifier_block *self, unsigned long cmd, void *v) +{ + int i; + + for (i = 0; i< MAX_GIC_NR; i++) { + switch (cmd) { + case CPU_PM_ENTER: + gic_cpu_save(i); On OMAP, GIC cpu interface context is lost only when CPU cluster is powered down. Yes, it's true, but that's the only chance we have to save the GIC CPU IF state if the GIC context is lost, right ? It is a private memory map per processor; I agree, it might be useless if just one CPU is shutdown, but at that point in time you do not know the state of other CPUs. If the cluster moves to a state where GIC context is lost at least you had the GIC CPU IF state saved. If we do not save it, well, there is no way to do that anymore since the last CPU cannot access other CPUs GIC CPU IF registers (or better, banked GIC distributor registers). If you force hotplug on CPUs other than 0 (that's the way it is done on OMAP4 in cpuidle, right ?) to hit deep low-power states you reinit the GIC CPU IF state as per cold boot, so yes, it is useless there. Actually, on OMAP there is no need to save any CPU interface registers. For my OMAP4 PM rebasing, for time-being I will go with exported GIC functions so that I don't have too many redundancies with GIC save/restore code. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH] ARM: do not mark CPU 0 as hotpluggable
On 7/21/2011 7:00 PM, Russell King - ARM Linux wrote: On Thu, Jul 21, 2011 at 11:03:04AM +0530, Santosh Shilimkar wrote: Just talking on behalf of OMAP, we can't offline CPU0 and limitation would remain in future OMAPs too. Apart from the broken IRQ migration, and the way CPU0 immediately reawakes if it is offlined on OMAP4 (even when IRQs are migrated off CPU0) because omap_read_auxcoreboot0() returns 0, is there any other reason? With fixed IRQ migration and forcing CPU0 into an infinite WFI loop, I can offline CPU0 and still have a running system. The secure software runs only on CPU0 and that's the biggest limitation. Other issues like hand-shake as you pointed out, power sequencing etc can be worked around. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 03/17] ARM: gic: Use cpu pm notifiers to save gic state
On 7/22/2011 12:36 AM, Colin Cross wrote: On Thu, Jul 21, 2011 at 3:46 AM, Santosh Shilimkar wrote: On 7/21/2011 3:57 PM, Lorenzo Pieralisi wrote: On Thu, Jul 21, 2011 at 09:32:12AM +0100, Santosh Shilimkar wrote: Lorenzo, Colin, On 7/7/2011 9:20 PM, Lorenzo Pieralisi wrote: From: Colin Cross When the cpu is powered down in a low power mode, the gic cpu interface may be reset, and when the cpu complex is powered down, the gic distributor may also be reset. This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save and restore the gic cpu interface registers, and the CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save and restore the gic distributor registers. Signed-off-by: Colin Cross --- arch/arm/common/gic.c | 212 + 1 files changed, 212 insertions(+), 0 deletions(-) diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c index 4ddd0a6..8d62e07 100644 --- a/arch/arm/common/gic.c +++ b/arch/arm/common/gic.c [...] I missed one more comment in the last review. +static int gic_notifier(struct notifier_block *self, unsigned long cmd, void *v) +{ + int i; + + for (i = 0; i< MAX_GIC_NR; i++) { + switch (cmd) { + case CPU_PM_ENTER: + gic_cpu_save(i); On OMAP, GIC cpu interface context is lost only when CPU cluster is powered down. Yes, it's true, but that's the only chance we have to save the GIC CPU IF state if the GIC context is lost, right ? It is a private memory map per processor; I agree, it might be useless if just one CPU is shutdown, but at that point in time you do not know the state of other CPUs. If the cluster moves to a state where GIC context is lost at least you had the GIC CPU IF state saved. If we do not save it, well, there is no way to do that anymore since the last CPU cannot access other CPUs GIC CPU IF registers (or better, banked GIC distributor registers). If you force hotplug on CPUs other than 0 (that's the way it is done on OMAP4 in cpuidle, right ?) to hit deep low-power states you reinit the GIC CPU IF state as per cold boot, so yes, it is useless there. Actually, on OMAP there is no need to save any CPU interface registers. For my OMAP4 PM rebasing, for time-being I will go with exported GIC functions so that I don't have too many redundancies with GIC save/restore code. I think you should try to balance cpu idle latency with reuse of common code. In this case, you are avoiding restoring 7 registers by reimplementing the bare minimum that is necessary for OMAP4, which is unlikely to make a measurable impact on wakeup latency. Can you try starting with reusing all the common code, and add some timestamps during wakeup to measure where the longest delays are, to determine where you should diverge from the common code and use omap-optimized code? I am going to use all the common code but having them exported functions gives more flexibility to call them in right and needed places. As discussed earlier, I plan to use the common GIC code wherever it's needed on OMAP. My main point was we are saving and restoring GIC CPU interface registers for a case where they are actually not lost. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH] ARM: do not mark CPU 0 as hotpluggable
On 7/22/2011 6:15 PM, Woodruff, Richard wrote: From: linux-arm-kernel-boun...@lists.infradead.org [mailto:linux-arm- kernel-boun...@lists.infradead.org] On Behalf Of Shilimkar, Santosh With fixed IRQ migration and forcing CPU0 into an infinite WFI loop, I can offline CPU0 and still have a running system. The secure software runs only on CPU0 and that's the biggest limitation. Other issues like hand-shake as you pointed out, power sequencing etc can be worked around. As you know well some of the secure APIs work on CPU1 and others do not. I notice in other thread Russell made assumption about CPU1 being able to use all because it could run some. This is not the case. I clarified that on the other thread. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC 2/6] sched: add a new SD SHARE_POWERLINE flag for sched_domain
Vincent, Few comments/questions. On Sunday 07 October 2012 01:13 PM, Vincent Guittot wrote: This new flag SD SHARE_POWERLINE reflects the sharing of the power rail between the members of a domain. As this is the current assumption of the scheduler, the flag is added to all sched_domain Signed-off-by: Vincent Guittot --- arch/ia64/include/asm/topology.h |1 + arch/tile/include/asm/topology.h |1 + include/linux/sched.h|1 + include/linux/topology.h |3 +++ kernel/sched/core.c |5 + 5 files changed, 11 insertions(+) diff --git a/arch/ia64/include/asm/topology.h b/arch/ia64/include/asm/topology.h index a2496e4..065c720 100644 --- a/arch/ia64/include/asm/topology.h +++ b/arch/ia64/include/asm/topology.h @@ -65,6 +65,7 @@ void build_cpu_to_node_map(void); | SD_BALANCE_EXEC \ | SD_BALANCE_FORK \ | SD_WAKE_AFFINE, \ + | arch_sd_share_power_line()\ .last_balance = jiffies, \ .balance_interval = 1,\ .nr_balance_failed = 0,\ diff --git a/arch/tile/include/asm/topology.h b/arch/tile/include/asm/topology.h index 7a7ce39..d39ed0b 100644 --- a/arch/tile/include/asm/topology.h +++ b/arch/tile/include/asm/topology.h @@ -72,6 +72,7 @@ static inline const struct cpumask *cpumask_of_node(int node) | 0*SD_PREFER_LOCAL \ | 0*SD_SHARE_CPUPOWER \ | 0*SD_SHARE_PKG_RESOURCES \ + | arch_sd_share_power_line()\ | 0*SD_SERIALIZE\ , \ .last_balance = jiffies, \ diff --git a/include/linux/sched.h b/include/linux/sched.h index 4786b20..74f2daf 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -862,6 +862,7 @@ enum cpu_idle_type { #define SD_WAKE_AFFINE0x0020 /* Wake task to waking CPU */ #define SD_PREFER_LOCAL 0x0040 /* Prefer to keep tasks local to this domain */ #define SD_SHARE_CPUPOWER 0x0080 /* Domain members share cpu power */ +#define SD_SHARE_POWERLINE 0x0100 /* Domain members share power domain */ If you ignore the current use of SD_SHARE_CPUPOWER, isn't the meaning of CPUPOWER and POWERLINE is same here. Just trying to understand the clear meaning of this new flag. Have you not considered SD_SHARE_CPUPOWER because it is being used for cpu_power and needs at least minimum two domains ? SD_PACKING would have been probably more appropriate based on the way it is being used in further series. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC 3/6] sched: pack small tasks
Vincent, Few comments/questions. On Sunday 07 October 2012 01:13 PM, Vincent Guittot wrote: During sched_domain creation, we define a pack buddy CPU if available. On a system that share the powerline at all level, the buddy is set to -1 On a dual clusters / dual cores system which can powergate each core and cluster independantly, the buddy configuration will be : | CPU0 | CPU1 | CPU2 | CPU3 | --- buddy | CPU0 | CPU0 | CPU0 | CPU2 | ^ Is that a typo ? Should it be CPU2 instead of CPU0 ? Small tasks tend to slip out of the periodic load balance. The best place to choose to migrate them is at their wake up. I have tried this series since I was looking at some of these packing bits. On Mobile workloads like OSIdle with Screen ON, MP3, gallary, I did see some additional filtering of threads with this series but its not making much difference in power. More on this below. Signed-off-by: Vincent Guittot --- kernel/sched/core.c |1 + kernel/sched/fair.c | 109 ++ kernel/sched/sched.h |1 + 3 files changed, 111 insertions(+) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index dab7908..70cadbe 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -6131,6 +6131,7 @@ cpu_attach_domain(struct sched_domain *sd, struct root_domain *rd, int cpu) rcu_assign_pointer(rq->sd, sd); destroy_sched_domains(tmp, cpu); + update_packing_domain(cpu); update_top_cache_domain(cpu); } diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 4f4a4f6..8c9d3ed 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -157,6 +157,63 @@ void sched_init_granularity(void) update_sysctl(); } + +/* + * Save the id of the optimal CPU that should be used to pack small tasks + * The value -1 is used when no buddy has been found + */ +DEFINE_PER_CPU(int, sd_pack_buddy); + +/* Look for the best buddy CPU that can be used to pack small tasks + * We make the assumption that it doesn't wort to pack on CPU that share the s/wort/worth + * same powerline. We looks for the 1st sched_domain without the + * SD_SHARE_POWERLINE flag. Then We look for the sched_group witht the lowest + * power per core based on the assumption that their power efficiency is + * better */ Commenting style.. /* * */ Can you please expand the why the assumption is right ? "it doesn't wort to pack on CPU that share the same powerline" Think about a scenario where you have quad core, ducal cluster system |Cluster1| |cluster 2| | CPU0 | CPU1 | CPU2 | CPU3 | | CPU0 | CPU1 | CPU2 | CPU3 | Both clusters run from same voltage rail and have same PLL clocking them. But the cluster have their own power domain and all CPU's can power gate them-self to low power states. Clusters also have their own level2 caches. In this case, you will still save power if you try to pack load on one cluster. No ? +void update_packing_domain(int cpu) +{ + struct sched_domain *sd; + int id = -1; + + sd = highest_flag_domain(cpu, SD_SHARE_POWERLINE); + if (!sd) + sd = rcu_dereference_check_sched_domain(cpu_rq(cpu)->sd); + else + sd = sd->parent; + + while (sd) { + struct sched_group *sg = sd->groups; + struct sched_group *pack = sg; + struct sched_group *tmp = sg->next; + + /* 1st CPU of the sched domain is a good candidate */ + if (id == -1) + id = cpumask_first(sched_domain_span(sd)); + + /* loop the sched groups to find the best one */ + while (tmp != sg) { + if (tmp->sgp->power * sg->group_weight < + sg->sgp->power * tmp->group_weight) + pack = tmp; + tmp = tmp->next; + } + + /* we have found a better group */ + if (pack != sg) + id = cpumask_first(sched_group_cpus(pack)); + + /* Look for another CPU than itself */ + if ((id != cpu) +|| ((sd->parent) && !(sd->parent->flags && SD_LOAD_BALANCE))) Is the condition "!(sd->parent->flags && SD_LOAD_BALANCE)" for big.LITTLE kind of system where SD_LOAD_BALANCE may not be used ? + break; + + sd = sd->parent; + } + + pr_info(KERN_INFO "CPU%d packing on CPU%d\n", cpu, id); + per_cpu(sd_pack_buddy, cpu) = id; +} + #if BITS_PER_LONG == 32 # define WMULT_CONST (~0UL) #else @@ -3073,6 +3130,55 @@ static int select_idle_sibling(struct task_struct *p, int target) return target; } +static inline bool is_buddy_busy(int cpu) +{ + struct rq *rq = cpu_rq(cpu); + + /* +* A busy buddy is a CPU with a high load or a small load with a lot of
Re: [RFC 4/6] sched: secure access to other CPU statistics
$subject is bit confusing here. On Sunday 07 October 2012 01:13 PM, Vincent Guittot wrote: The atomic update of runnable_avg_sum and runnable_avg_period are ensured by their size and the toolchain. But we must ensure to not read an old value for one field and a newly updated value for the other field. As we don't want to lock other CPU while reading these fields, we read twice each fields and check that no change have occured in the middle. Signed-off-by: Vincent Guittot --- kernel/sched/fair.c | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 8c9d3ed..6df53b5 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -3133,13 +3133,28 @@ static int select_idle_sibling(struct task_struct *p, int target) static inline bool is_buddy_busy(int cpu) { struct rq *rq = cpu_rq(cpu); + volatile u32 *psum = &rq->avg.runnable_avg_sum; + volatile u32 *pperiod = &rq->avg.runnable_avg_period; + u32 sum, new_sum, period, new_period; + int timeout = 10; So it can be 2 times read or more as well. + + while (timeout) { + sum = *psum; + period = *pperiod; + new_sum = *psum; + new_period = *pperiod; + + if ((sum == new_sum) && (period == new_period)) + break; + + timeout--; + } Seems like you did notice incorrect pair getting read for rq runnable_avg_sum and runnable_avg_period. Seems like the fix is to update them together under some lock to avoid such issues. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC 5/6] sched: pack the idle load balance
On Sunday 07 October 2012 01:13 PM, Vincent Guittot wrote: Look for an idle CPU close the pack buddy CPU whenever possible. s/close/close to The goal is to prevent the wake up of a CPU which doesn't share the power line of the pack CPU Signed-off-by: Vincent Guittot --- kernel/sched/fair.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 6df53b5..f76acdc 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -5158,7 +5158,25 @@ static struct { static inline int find_new_ilb(int call_cpu) { + struct sched_domain *sd; int ilb = cpumask_first(nohz.idle_cpus_mask); + int buddy = per_cpu(sd_pack_buddy, call_cpu); + + /* +* If we have a pack buddy CPU, we try to run load balance on a CPU +* that is close to the buddy. +*/ + if (buddy != -1) + for_each_domain(buddy, sd) { + if (sd->flags & SD_SHARE_CPUPOWER) + continue; Do you mean SD_SHARE_POWERLINE here ? + + ilb = cpumask_first_and(sched_domain_span(sd), + nohz.idle_cpus_mask); + + if (ilb < nr_cpu_ids) + break; + } if (ilb < nr_cpu_ids && idle_cpu(ilb)) return ilb; Can you please expand "idle CPU _close_ the pack buddy CPU" ? Regards santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC 6/6] ARM: sched: clear SD_SHARE_POWERLINE
On Sunday 07 October 2012 01:13 PM, Vincent Guittot wrote: The ARM platforms take advantage of packing small tasks on few cores. This is true even when the cores of a cluster can't be powergated independently. Signed-off-by: Vincent Guittot --- arch/arm/kernel/topology.c |5 + 1 file changed, 5 insertions(+) diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c index 26c12c6..00511d0 100644 --- a/arch/arm/kernel/topology.c +++ b/arch/arm/kernel/topology.c @@ -226,6 +226,11 @@ static inline void update_cpu_power(unsigned int cpuid, unsigned int mpidr) {} */ struct cputopo_arm cpu_topology[NR_CPUS]; +int arch_sd_share_power_line(void) +{ + return 0*SD_SHARE_POWERLINE; +} Making this selection of policy based on sched domain will better. Just gives the flexibility to choose a separate scheme for big and little systems which will be very convenient. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC 2/6] sched: add a new SD SHARE_POWERLINE flag for sched_domain
On Monday 29 October 2012 03:20 PM, Vincent Guittot wrote: It looks like i need to describe more what On 29 October 2012 10:40, Vincent Guittot wrote: On 24 October 2012 17:17, Santosh Shilimkar wrote: Vincent, Few comments/questions. On Sunday 07 October 2012 01:13 PM, Vincent Guittot wrote: This new flag SD SHARE_POWERLINE reflects the sharing of the power rail between the members of a domain. As this is the current assumption of the scheduler, the flag is added to all sched_domain Signed-off-by: Vincent Guittot --- arch/ia64/include/asm/topology.h |1 + arch/tile/include/asm/topology.h |1 + include/linux/sched.h|1 + include/linux/topology.h |3 +++ kernel/sched/core.c |5 + 5 files changed, 11 insertions(+) diff --git a/arch/ia64/include/asm/topology.h b/arch/ia64/include/asm/topology.h index a2496e4..065c720 100644 --- a/arch/ia64/include/asm/topology.h +++ b/arch/ia64/include/asm/topology.h @@ -65,6 +65,7 @@ void build_cpu_to_node_map(void); | SD_BALANCE_EXEC \ | SD_BALANCE_FORK \ | SD_WAKE_AFFINE, \ + | arch_sd_share_power_line()\ .last_balance = jiffies, \ .balance_interval = 1,\ .nr_balance_failed = 0,\ diff --git a/arch/tile/include/asm/topology.h b/arch/tile/include/asm/topology.h index 7a7ce39..d39ed0b 100644 --- a/arch/tile/include/asm/topology.h +++ b/arch/tile/include/asm/topology.h @@ -72,6 +72,7 @@ static inline const struct cpumask *cpumask_of_node(int node) | 0*SD_PREFER_LOCAL \ | 0*SD_SHARE_CPUPOWER \ | 0*SD_SHARE_PKG_RESOURCES \ + | arch_sd_share_power_line()\ | 0*SD_SERIALIZE\ , \ .last_balance = jiffies, \ diff --git a/include/linux/sched.h b/include/linux/sched.h index 4786b20..74f2daf 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -862,6 +862,7 @@ enum cpu_idle_type { #define SD_WAKE_AFFINE0x0020 /* Wake task to waking CPU */ #define SD_PREFER_LOCAL 0x0040 /* Prefer to keep tasks local to this domain */ #define SD_SHARE_CPUPOWER 0x0080 /* Domain members share cpu power */ +#define SD_SHARE_POWERLINE 0x0100 /* Domain members share power domain */ If you ignore the current use of SD_SHARE_CPUPOWER, isn't the meaning of CPUPOWER and POWERLINE is same here. Just trying to understand the clear meaning of this new flag. Have you not considered SD_SHARE_CPUPOWER because it is being used for cpu_power and needs at least minimum two domains ? SD_PACKING would have been probably more appropriate based on the way it is being used in further series. CPUPOWER reflects the share of hw ressources between cores like for hyper threading. POWERLINE describes the fact that cores are sharing the same power line amore precisely the powergate. Sorry, the mail has been sent too early while I was writing it CPUPOWER reflects the share of hw ressources between cores like for hyper threading. POWERLINE describes the fact that cores are sharing the same power line and more precisely the same power gating. It looks like I need to describe more precisely what i would mean with SHARE_POWERLINE. Yes. More description will help. I see bit of overlap POWERLINE flag with SD_SHARE_CPUPOWER and SD_SHARE_PKG_RESOURCES and hence the questions. I don't want to use PACKING because it's more a behavior than a feature. If cores can power gate independently (!SD_SHARE_POWERLINE), packing small tasks is one interesting behavior but it may be not the only one. I want to make a difference between the HW configuration and the behavior we want to have above it Fair enough. Thanks for clarification. Regards, Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC 3/6] sched: pack small tasks
On Monday 29 October 2012 06:42 PM, Vincent Guittot wrote: On 24 October 2012 17:20, Santosh Shilimkar wrote: Vincent, Few comments/questions. On Sunday 07 October 2012 01:13 PM, Vincent Guittot wrote: During sched_domain creation, we define a pack buddy CPU if available. On a system that share the powerline at all level, the buddy is set to -1 On a dual clusters / dual cores system which can powergate each core and cluster independantly, the buddy configuration will be : | CPU0 | CPU1 | CPU2 | CPU3 | --- buddy | CPU0 | CPU0 | CPU0 | CPU2 | ^ Is that a typo ? Should it be CPU2 instead of CPU0 ? No it's not a typo. The system packs at each scheduling level. It starts to pack in cluster because each core can power gate independently so CPU1 tries to pack its tasks in CPU0 and CPU3 in CPU2. Then, it packs at CPU level so CPU2 tries to pack in the cluster of CPU0 and CPU0 packs in itself I get it. Though in above example a task may migrate from say CPU3->CPU2->CPU0 as part of packing. I was just thinking whether moving such task from say CPU3 to CPU0 might be best instead. Small tasks tend to slip out of the periodic load balance. The best place to choose to migrate them is at their wake up. I have tried this series since I was looking at some of these packing bits. On Mobile workloads like OSIdle with Screen ON, MP3, gallary, I did see some additional filtering of threads with this series but its not making much difference in power. More on this below. Can I ask you which configuration you have used ? how many cores and cluster ? Can they be power gated independently ? I have been trying with couple of setups. Dual Core ARM machine and Quad core X86 box with single package thought most of the mobile workload analysis I was doing on ARM machine. On both setups CPUs can be gated independently. Signed-off-by: Vincent Guittot --- kernel/sched/core.c |1 + kernel/sched/fair.c | 109 ++ kernel/sched/sched.h |1 + 3 files changed, 111 insertions(+) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index dab7908..70cadbe 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -6131,6 +6131,7 @@ cpu_attach_domain(struct sched_domain *sd, struct root_domain *rd, int cpu) rcu_assign_pointer(rq->sd, sd); destroy_sched_domains(tmp, cpu); + update_packing_domain(cpu); update_top_cache_domain(cpu); } diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 4f4a4f6..8c9d3ed 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -157,6 +157,63 @@ void sched_init_granularity(void) update_sysctl(); } + +/* + * Save the id of the optimal CPU that should be used to pack small tasks + * The value -1 is used when no buddy has been found + */ +DEFINE_PER_CPU(int, sd_pack_buddy); + +/* Look for the best buddy CPU that can be used to pack small tasks + * We make the assumption that it doesn't wort to pack on CPU that share the s/wort/worth yes + * same powerline. We looks for the 1st sched_domain without the + * SD_SHARE_POWERLINE flag. Then We look for the sched_group witht the lowest + * power per core based on the assumption that their power efficiency is + * better */ Commenting style.. /* * */ yes Can you please expand the why the assumption is right ? "it doesn't wort to pack on CPU that share the same powerline" By "share the same power-line", I mean that the CPUs can't power off independently. So if some CPUs can't power off independently, it's worth to try to use most of them to race to idle. In that case I suggest we use different word here. Power line can be treated as voltage line, power domain. May be SD_SHARE_CPU_POWERDOMAIN ? Think about a scenario where you have quad core, ducal cluster system |Cluster1| |cluster 2| | CPU0 | CPU1 | CPU2 | CPU3 | | CPU0 | CPU1 | CPU2 | CPU3 | Both clusters run from same voltage rail and have same PLL clocking them. But the cluster have their own power domain and all CPU's can power gate them-self to low power states. Clusters also have their own level2 caches. In this case, you will still save power if you try to pack load on one cluster. No ? yes, I need to update the description of SD_SHARE_POWERLINE because I'm afraid I was not clear enough. SD_SHARE_POWERLINE includes the power gating capacity of each core. For your example above, the SD_SHARE_POWERLINE shoud be cleared at both MC and CPU level. Thanks for clarification. +void update_packing_domain(int cpu) +{ + struct sched_domain *sd; + int id = -1; + + sd = highest_flag_domain(cpu, SD_SHARE_POWERLINE); + if (!sd) + sd = rcu_dereference_check_sched_domain(cpu_rq(cpu)->sd); + else + sd = sd->
Re: [RFC 5/6] sched: pack the idle load balance
On Monday 29 October 2012 06:57 PM, Vincent Guittot wrote: On 24 October 2012 17:21, Santosh Shilimkar wrote: On Sunday 07 October 2012 01:13 PM, Vincent Guittot wrote: Look for an idle CPU close the pack buddy CPU whenever possible. s/close/close to yes The goal is to prevent the wake up of a CPU which doesn't share the power line of the pack CPU Signed-off-by: Vincent Guittot --- kernel/sched/fair.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 6df53b5..f76acdc 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -5158,7 +5158,25 @@ static struct { static inline int find_new_ilb(int call_cpu) { + struct sched_domain *sd; int ilb = cpumask_first(nohz.idle_cpus_mask); + int buddy = per_cpu(sd_pack_buddy, call_cpu); + + /* +* If we have a pack buddy CPU, we try to run load balance on a CPU +* that is close to the buddy. +*/ + if (buddy != -1) + for_each_domain(buddy, sd) { + if (sd->flags & SD_SHARE_CPUPOWER) + continue; Do you mean SD_SHARE_POWERLINE here ? No, I just don't want to take hyperthread level for ILB + + ilb = cpumask_first_and(sched_domain_span(sd), + nohz.idle_cpus_mask); + + if (ilb < nr_cpu_ids) + break; + } if (ilb < nr_cpu_ids && idle_cpu(ilb)) return ilb; Can you please expand "idle CPU _close_ the pack buddy CPU" ? The goal is to packed the tasks on the pack buddy CPU so when the scheduler needs to start ILB, I try to wake up a CPU that is close to the buddy and preferably in the same cluster I see your point now. Thanks for clarification. Regards santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC 6/6] ARM: sched: clear SD_SHARE_POWERLINE
On Monday 29 October 2012 06:58 PM, Vincent Guittot wrote: On 24 October 2012 17:21, Santosh Shilimkar wrote: On Sunday 07 October 2012 01:13 PM, Vincent Guittot wrote: The ARM platforms take advantage of packing small tasks on few cores. This is true even when the cores of a cluster can't be powergated independently. Signed-off-by: Vincent Guittot --- arch/arm/kernel/topology.c |5 + 1 file changed, 5 insertions(+) diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c index 26c12c6..00511d0 100644 --- a/arch/arm/kernel/topology.c +++ b/arch/arm/kernel/topology.c @@ -226,6 +226,11 @@ static inline void update_cpu_power(unsigned int cpuid, unsigned int mpidr) {} */ struct cputopo_arm cpu_topology[NR_CPUS]; +int arch_sd_share_power_line(void) +{ + return 0*SD_SHARE_POWERLINE; +} Making this selection of policy based on sched domain will better. Just gives the flexibility to choose a separate scheme for big and little systems which will be very convenient. I agree that it would be more flexible to be able to set it for each level Will you be addressing that in next version then ? Regards santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: Question regarding broadcast timer/cpuidle and /proc/interrupts
On Monday 21 January 2013 07:47 PM, Daniel Lezcano wrote: Hi All, I have a question regarding the behavior of cpuidle on pandaboard. 1. cpuidle is enabled 2. The deep idle states seem to be reach for i in $(find /sys/devices/system/cpu -name "usage"); do echo "$i : $(cat $i)"; done /sys/devices/system/cpu/cpu0/cpuidle/state0/usage : 7049 /sys/devices/system/cpu/cpu0/cpuidle/state1/usage : 17 /sys/devices/system/cpu/cpu0/cpuidle/state2/usage : 1341 /sys/devices/system/cpu/cpu1/cpuidle/state0/usage : 6318 /sys/devices/system/cpu/cpu1/cpuidle/state1/usage : 17 /sys/devices/system/cpu/cpu1/cpuidle/state2/usage : 1341 3. Regarding the cpuidle driver code : the "state1" and "state2" are coupled states where the broadcast timer is used instead of the local timer. I assume this is because they go down when we reach these idle states. Thats correct. Local timer are not wakeup capable and hence we switch to a wakeup capable broadcast timer. 4. The content of /proc/interrupts shows no broadcast timer used at all. ... IPI1: 0 0 Timer broadcast interrupts ... Shouldn't be the broadcast timer used sometimes ? or did I miss something ? There might be an issue with status updating. Just look for gptimer1 interrupts. if they are incrementing then, broadcast is being used but just the status update isn't happening some how. regards santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 3/3] cpufreq: Remove unnecessary use of policy->shared_type
On Friday 01 February 2013 12:10 PM, Viresh Kumar wrote: policy->shared_type field was added only for SoCs with ACPI support: commit 3b2d99429e3386b6e2ac949fc72486509c8bbe36 Author: Venkatesh Pallipadi Date: Wed Dec 14 15:05:00 2005 -0500 P-state software coordination for ACPI core http://bugzilla.kernel.org/show_bug.cgi?id=5737 Many non-ACPI systems are filling this field by mistake, which makes its usage confusing. Lets clean it. Signed-off-by: Viresh Kumar Cc: Linus Walleij Cc: Stephen Warren Cc: Shawn Guo Cc: Santosh Shilimkar --- I haven't looked at the cpufreq code recently but remember that it was needed to ensure that all the CPU which share clock/voltage gets updated (affected cpus) on freq change. The CPUs which needs SW co-ordination, should have this flag enabled and OMAP was falling in that category. May be I miss-understood its use, but can you confirm that SW co-ordination logic continues to work without this flag ? Regards, Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 3/3] cpufreq: Remove unnecessary use of policy->shared_type
On Friday 01 February 2013 12:43 PM, Viresh Kumar wrote: On 1 February 2013 12:17, Santosh Shilimkar wrote: I haven't looked at the cpufreq code recently but remember that it was needed to ensure that all the CPU which share clock/voltage gets updated (affected cpus) on freq change. The CPUs which needs SW co-ordination, should have this flag enabled and OMAP was falling in that category. Freq change are done by the target routines of platform cpufreq drivers and they do something like: for_each_cpu(freqs.cpu, policy->cpus) cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE); The only requirement from cpufreq core is to keep cpus sharing clock in policy->cpus. I am not talking about just notifiers. This is for external users who has subscribed for notifiers. The point is whether the core CPUFReq gets updated without that flag for all affected CPU. May be I miss-understood its use, but can you confirm that SW co-ordination logic continues to work without this flag ? I believe it should work. It works for the systems i worked on: SPEAr13xx: Dual Cortex A9 ARM TC2: two clusters of A15s and A7s. I will give a try some time next week on OMAP. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 3/3] cpufreq: Remove unnecessary use of policy->shared_type
On Friday 01 February 2013 01:32 PM, Viresh Kumar wrote: On 1 February 2013 13:03, Santosh Shilimkar wrote: I am not talking about just notifiers. This is for external users who has subscribed for notifiers. The point is whether the core CPUFReq gets updated without that flag for all affected CPU. Yes, its safe. Follow this thread, yesterday i explained this to Tomasz Figa: http://www.spinics.net/lists/arm-kernel/msg221629.html That part was very clear to me Viresh. Anyway thanks for the link. From what I read so far, it might just work but I would want to try it out before acking the approach. Regards, Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH 3/3] cpufreq: Remove unnecessary use of policy->shared_type
Viresh, On Friday 01 February 2013 02:22 PM, Santosh Shilimkar wrote: On Friday 01 February 2013 01:32 PM, Viresh Kumar wrote: On 1 February 2013 13:03, Santosh Shilimkar wrote: I am not talking about just notifiers. This is for external users who has subscribed for notifiers. The point is whether the core CPUFReq gets updated without that flag for all affected CPU. Yes, its safe. Follow this thread, yesterday i explained this to Tomasz Figa: http://www.spinics.net/lists/arm-kernel/msg221629.html That part was very clear to me Viresh. Anyway thanks for the link. From what I read so far, it might just work but I would want to try it out before acking the approach. You are correct. Sorry for oversight on your initial point about the usage of the flag. When I added that flag, I just went by the description thinking the cpufreq core booking and stat updates use the flag. Its not the case. Thanks for the fix. For the patch, Acked-by: Santosh Shilimkar ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [resend] Timer broadcast question
On Tuesday 19 February 2013 11:51 PM, Daniel Lezcano wrote: On 02/19/2013 07:10 PM, Thomas Gleixner wrote: On Tue, 19 Feb 2013, Daniel Lezcano wrote: I am working on identifying the different wakeup sources from the interrupts and I have a question regarding the timer broadcast. The broadcast timer is setup to the next event and that will wake up any idle cpu belonging to the "broadcast cpumask", right ? The cpu which has been woken up will look for each cpu the next-event and send an IPI to wake it up. Although, it is possible the sender of this IPI may not be concerned by the timer expiration and has been woken up just for sending the IPI, right ? Correct. If this is correct, is it possible to setup the timer irq affinity to a cpu which will be concerned by the timer expiration ? so we prevent an unnecessary wake up for a cpu. It is possible, but we never implemented it. If we go there, we want to make that conditional on a property flag, because some interrupt controllers especially on x86 only allow to move the affinity from interrupt context, which is pointless. Thanks Thomas for your quick answer. I will write a RFC patchset. Last year I implemented the affinity hook for broad-cast code and experimented with it. Since the system I was using was dual core, it wasn't much beneficial and hence gave up later. I did remember discussing the approach with few folks in the conference. Patch in the end of the email (also attached) for generic broadcast code. I didn't look at all corner case though. In arch code then you need to setup "broadcast_affinity" hook which should be able to get handle of the arch irqchip and call the respective affinity handler. Just 3 lines function should do the trick. As Thomas said, effectiveness of such optimization solely depends on how well the affinity (in low powers) supported by your IRQ chip. Hope this is helpful for you. Regards, Santosh From d70f2d48ec08a3f1d73187c49b16e4e60f81a50c Mon Sep 17 00:00:00 2001 From: Santosh Shilimkar Date: Wed, 25 Jul 2012 03:42:33 +0530 Subject: [PATCH] tick-broadcast: Add tick road-cast affinity suport Current tick broad-cast code has affinity set to the boot CPU and hence the boot CPU will always wakeup from low power states when broad cast timer is armed even if the next expiry event doesn't belong to it. Patch adds broadcast affinity functionality to avoid above and let the tick framework set the affinity of the event for the CPU it belongs. Signed-off-by: Santosh Shilimkar --- include/linux/clockchips.h |2 ++ kernel/time/tick-broadcast.c | 13 - 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h index 8a7096f..5488cdc 100644 --- a/include/linux/clockchips.h +++ b/include/linux/clockchips.h @@ -95,6 +95,8 @@ struct clock_event_device { unsigned long retries; void(*broadcast)(const struct cpumask *mask); + void(*broadcast_affinity) + (const struct cpumask *mask, int irq); void(*set_mode)(enum clock_event_mode mode, struct clock_event_device *); void(*suspend)(struct clock_event_device *); diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c index f113755..2ec2425 100644 --- a/kernel/time/tick-broadcast.c +++ b/kernel/time/tick-broadcast.c @@ -39,6 +39,8 @@ static void tick_broadcast_clear_oneshot(int cpu); static inline void tick_broadcast_clear_oneshot(int cpu) { } #endif +static inline void dummy_broadcast_affinity(const struct cpumask *mask, + int irq) { } /* * Debugging: see timer_list.c */ @@ -485,14 +487,19 @@ void tick_broadcast_oneshot_control(unsigned long reason) if (!cpumask_test_cpu(cpu, tick_get_broadcast_oneshot_mask())) { cpumask_set_cpu(cpu, tick_get_broadcast_oneshot_mask()); clockevents_set_mode(dev, CLOCK_EVT_MODE_SHUTDOWN); - if (dev->next_event.tv64 < bc->next_event.tv64) + if (dev->next_event.tv64 < bc->next_event.tv64) { tick_broadcast_set_event(dev->next_event, 1); + bc->broadcast_affinity( + tick_get_broadcast_oneshot_mask(), bc->irq); + } } } else { if (cpumask_test_cpu(cpu, tick_get_broadcast_oneshot_mask())) { cpumask_clear_cpu(cpu, tick_get_broadcast_oneshot_mask()); clockevents_set_mode(dev, CLOCK_EVT_MODE_ONESHOT); + bc->broadcast_affinity( +
Re: [resend] Timer broadcast question
On Thursday 21 February 2013 02:31 PM, Daniel Lezcano wrote: On 02/21/2013 07:19 AM, Santosh Shilimkar wrote: On Tuesday 19 February 2013 11:51 PM, Daniel Lezcano wrote: On 02/19/2013 07:10 PM, Thomas Gleixner wrote: On Tue, 19 Feb 2013, Daniel Lezcano wrote: I am working on identifying the different wakeup sources from the interrupts and I have a question regarding the timer broadcast. The broadcast timer is setup to the next event and that will wake up any idle cpu belonging to the "broadcast cpumask", right ? The cpu which has been woken up will look for each cpu the next-event and send an IPI to wake it up. Although, it is possible the sender of this IPI may not be concerned by the timer expiration and has been woken up just for sending the IPI, right ? Correct. If this is correct, is it possible to setup the timer irq affinity to a cpu which will be concerned by the timer expiration ? so we prevent an unnecessary wake up for a cpu. It is possible, but we never implemented it. If we go there, we want to make that conditional on a property flag, because some interrupt controllers especially on x86 only allow to move the affinity from interrupt context, which is pointless. Thanks Thomas for your quick answer. I will write a RFC patchset. Last year I implemented the affinity hook for broad-cast code and experimented with it. Since the system I was using was dual core, it wasn't much beneficial and hence gave up later. I did remember discussing the approach with few folks in the conference. I did a brief test with a similar patch on a ARM u8500 board. The timer is tied with CPU0 by default, setting the dynamic irq affinity reduce considerably the number of IPI. The difference with your patch is the affinity is set to one CPU, the first one which is supposed to be wake up by the timer expiration. This is easy to spot with a small program doing usleep wired on CPU1. We see CPU0 waking up to send an IPI to CPU1 and going to idle again. I don't know how that behaves with OMAP4 with this patch (which I guess it is the board you used), but the coupled idle state traces could be ambiguous if you relied on it to check the benefit of this patch. Across OMAP4 and OMAP5 based devices, only the general purpose OMAP5 devices the approach was useful. Rest of the devices had constraints of master CPU(CPU0) waking up first always which in turns means pining the affinity to that CPU always which the current code already does. That was also another reason I didn't persue it further. IMO, it is worth to implement such solution and perhaps we can extend it to optimize the package idle time with the generic power domain tied with the irq. Anyway, it is a random thought let's see that later :) It is surely a good optimization especially for multi-core CPUIdle. Patch in the end of the email (also attached) for generic broadcast code. I didn't look at all corner case though. In arch code then you need to setup "broadcast_affinity" hook which should be able to get handle of the arch irqchip and call the respective affinity handler. Just 3 lines function should do the trick. As Thomas said, effectiveness of such optimization solely depends on how well the affinity (in low powers) supported by your IRQ chip. Hope this is helpful for you. Thanks a lot for your patch and your feedbacks. Am glad that it was helpful. Regards, Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: backup/restore concept?
On Wednesday 24 July 2013 01:21 PM, Nishanth Menon wrote: > On 07/24/2013 12:49 AM, Viresh Kumar wrote: >> Adding more relevant list in cc. >> >> On 23 July 2013 16:05, Ryan wrote: >>> Hi, >>> >>> I have some doubts on backup and restore operation. From what i understand: >>> >>> We copy all registers values & addresses of all controllers in the SOC >>> to the internal RAM or SRAM. >>> before we put CPU to sleep? >>> >>> I want to know if we also copy the code segment into SRAM and what >>> happens after wakeup. >>> If so, where exactly we need to copy and how cpu jumps here after >>> wakeup. or is there any other mechanism >>> that is used. What executes first since DDR is in self-refresh. >>> >>> I use OMAP4 and this is my understanding. I could not understand much >>> other than in OFF mode >>> all the controller registers get copied to SRAM. Does anything else >>> also gets copied too? >>> or am i missing any basics here. > This understanding is not accurate. OMAP behavior for "OFF mode" is as > follows (as part of suspend/resume): > - drivers do their own "context save" - saving of registers based on their > need. > - SAR registers are saved (note - this is *not* every possible register on > OMAP - but a core subset). > - cpu goes to WFI triggering h/w statemachine flow. (wfi instruction is in > DDR) >- as part of "OFF mode" DDR is put into self refresh automatically by > memory controller. > > on wakeup > - core registers are restored by hardware > - DDR is brought out of selfrefresh, > - execution resume in resume function pointer > - drivers restore their own modules as needed. > > So, there is no real black magic here :) > :) Also if you are interested in how the SRAM copy stuff work, look at OMAP3 entry into OFF state and memory self refresh is triggered from code running from SRAM. On the wakeup though, we directly jump to DDR address. regards, Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
RE: [PATCH v2 2/3] ARM: omap4: Correct definition of do_wfi() forCONFIG_THUMB2_KERNEL
Dave, > -Original Message- > From: linaro-dev-boun...@lists.linaro.org [mailto:linaro-dev- > boun...@lists.linaro.org] On Behalf Of Dave Martin > Sent: Monday, December 06, 2010 11:06 PM > To: linux-arm-ker...@lists.infradead.org > Cc: Tony Lindgren; Dave Martin; linux-o...@vger.kernel.org; linaro- > d...@lists.linaro.org > Subject: [PATCH v2 2/3] ARM: omap4: Correct definition of do_wfi() > forCONFIG_THUMB2_KERNEL > > For the Thumb-2 case, the "wfi" mnemonic is used, since in this > case the tools will necessarily be new enough to support it. > > Signed-off-by: Dave Martin > --- > KernelVersion: 2.6.37-rc4 The choice of opcode instead of instruction here was not because of toolchain. The problem was it breaks multi-omap build where ARMv6 and ARMv7 are build together. For this reason I NAK this patch. > > arch/arm/mach-omap2/include/mach/omap4-common.h |5 + > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/mach-omap2/include/mach/omap4-common.h > b/arch/arm/mach-omap2/include/mach/omap4-common.h > index 2744dfe..c6b1320 100644 > --- a/arch/arm/mach-omap2/include/mach/omap4-common.h > +++ b/arch/arm/mach-omap2/include/mach/omap4-common.h > @@ -17,8 +17,13 @@ > * wfi used in low power code. Directly opcode is used instead > * of instruction to avoid mulit-omap build break > */ > +#ifdef CONFIG_THUMB2_KERNEL > +#define do_wfi() \ > + __asm__ __volatile__ ("wfi" : : : "memory") > +#else > #define do_wfi() \ > __asm__ __volatile__ (".word0xe320f003" : : : "memory") > +#endif > > #ifdef CONFIG_CACHE_L2X0 > extern void __iomem *l2cache_base; > -- > 1.7.1 > > > ___ > linaro-dev mailing list > linaro-dev@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-dev ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
RE: [PATCH v2 3/3] ARM: omap4: Convert END() to ENDPROC() for correctlinkage with CONFIG_THUMB2_KERNEL
> -Original Message- > From: linaro-dev-boun...@lists.linaro.org [mailto:linaro-dev- > boun...@lists.linaro.org] On Behalf Of Dave Martin > Sent: Monday, December 06, 2010 11:06 PM > To: linux-arm-ker...@lists.infradead.org > Cc: Tony Lindgren; Dave Martin; linux-o...@vger.kernel.org; linaro- > d...@lists.linaro.org > Subject: [PATCH v2 3/3] ARM: omap4: Convert END() to ENDPROC() for > correctlinkage with CONFIG_THUMB2_KERNEL > > almost all code for v7+ platforms) is deprecated/incorrect. > > ENDPROC() tags the affected symbol as a function symbol, which will > ensure that link-time fixups don't accidentally switch to the > wrong instruction set. > > omap_secondary_startup might still need to be changed to ARM, > depending on the compatibility status of bootloaders. > > Signed-off-by: Dave Martin > --- > KernelVersion: 2.6.37-rc4 Looks ok to me. Acked-by: Santosh Shilimkar > > arch/arm/mach-omap2/omap-headsmp.S |2 +- > arch/arm/mach-omap2/omap44xx-smc.S |8 > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/mach-omap2/omap-headsmp.S b/arch/arm/mach- > omap2/omap-headsmp.S > index 6ae937a..4ee6aec 100644 > --- a/arch/arm/mach-omap2/omap-headsmp.S > +++ b/arch/arm/mach-omap2/omap-headsmp.S > @@ -45,5 +45,5 @@ hold: ldr r12,=0x103 >* should now contain the SVC stack for this core >*/ > b secondary_startup > -END(omap_secondary_startup) > +ENDPROC(omap_secondary_startup) > > diff --git a/arch/arm/mach-omap2/omap44xx-smc.S b/arch/arm/mach- > omap2/omap44xx-smc.S > index 1980dc3..e69d37d 100644 > --- a/arch/arm/mach-omap2/omap44xx-smc.S > +++ b/arch/arm/mach-omap2/omap44xx-smc.S > @@ -29,7 +29,7 @@ ENTRY(omap_smc1) > dsb > smc #0 > ldmfd sp!, {r2-r12, pc} > -END(omap_smc1) > +ENDPROC(omap_smc1) > > ENTRY(omap_modify_auxcoreboot0) > stmfd sp!, {r1-r12, lr} > @@ -37,7 +37,7 @@ ENTRY(omap_modify_auxcoreboot0) > dsb > smc #0 > ldmfd sp!, {r1-r12, pc} > -END(omap_modify_auxcoreboot0) > +ENDPROC(omap_modify_auxcoreboot0) > > ENTRY(omap_auxcoreboot_addr) > stmfd sp!, {r2-r12, lr} > @@ -45,7 +45,7 @@ ENTRY(omap_auxcoreboot_addr) > dsb > smc #0 > ldmfd sp!, {r2-r12, pc} > -END(omap_auxcoreboot_addr) > +ENDPROC(omap_auxcoreboot_addr) > > ENTRY(omap_read_auxcoreboot0) > stmfd sp!, {r2-r12, lr} > @@ -54,4 +54,4 @@ ENTRY(omap_read_auxcoreboot0) > smc #0 > mov r0, r0, lsr #9 > ldmfd sp!, {r2-r12, pc} > -END(omap_read_auxcoreboot0) > +ENDPROC(omap_read_auxcoreboot0) > -- > 1.7.1 > > > ___ > linaro-dev mailing list > linaro-dev@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-dev ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
RE: [PATCH v2 1/3] ARM: omap: Enable low-level omap3 PM code to work withCONFIG_THUMB2_KERNEL
Dave, > -Original Message- > From: linaro-dev-boun...@lists.linaro.org [mailto:linaro-dev- > boun...@lists.linaro.org] On Behalf Of Dave Martin > Sent: Monday, December 06, 2010 11:06 PM > To: linux-arm-ker...@lists.infradead.org > Cc: Tony Lindgren; Dave Martin; linux-o...@vger.kernel.org; linaro- > d...@lists.linaro.org > Subject: [PATCH v2 1/3] ARM: omap: Enable low-level omap3 PM code to work > withCONFIG_THUMB2_KERNEL > > sleep34xx.S, sram34xx.S: > > * Added ENDPROC() directives for all exported function symbols. > Without these, exported function symbols are not correctly > identified as Thumb by the linker, causing incorrect linkage. > This is needed to avoid some calls to the functions ending up > with the CPU in the wrong instruction set. > > * Added .align directives where needed to ensure that .word won't > be misaligned. (Note that ENTRY() implies .align; no extra > .align has been added for these cases.) > > * Exported new "base address" symbols for functions which get > copied to sram by code outside sleep34xx.S (applies to > save_secure_ram_context and omap32xx_cpu_suspend), and fix up > the relevant address arithmetic so that these will be copied > and called correctly by the Thumb code in the rest of the > kernel. > > * Explicitly build a few parts of sleep34xx.S as ARM. > > * lock_scratchpad_sem is kept as ARM because of the need to > synchronise with hardware (?) using the SWP instruction. > > * save_secure_ram_context and omap34xx_cpu_suspend are built > as ARM in case the Secure World firmware expects to decode > the comment field from the SMC (aka smi) instructions. > > This can be undone later if the firmware is confirmed as > able to decode the Thumb SMC encoding (or ignores the > comment field). > > * es3_sdrc_fix should presumably only be called from the > low-level wakeup code. To minimise the diff, switched this > to ARM and demoted it to be a local symbol, since I believe > it shouldn't be called from outside anyway. > > To prevent future maintainence problems, it would probably be > a good idea to demote _all_ functions which aren't called > externally to local. (i.e., everything except for > get_es3_restore_pointer, get_restore_pointer, > omap34xx_cpu_suspend and save_secure_ram_context). > > For now, I've left these as-is to minimise disruption. > >* Use a separate base register instead of PC-relative stores in > sram34xx.S. This isn't permitted in Thumb-2, but at least > some versions of gas will silently output non-working code, > leading to unpredictable run-time behaviour. > > pm34xx.c, pm.h, sram.c, sram.h: > > * Resolve some memory addressing issues where a function symbol's > value is assumed to be equal to the start address of the > function body for the purpose of copying some low-level code > from sleep34xx.S to SRAM. > > This assumption breaks for Thumb, since Thumb functions symbols > have bit 0 set to indicate the Thumb-ness. This would result > in a non-working, off-by-one copy of the function body. > > Tested on Beagle xM A2: > * CONFIG_THUMB2_KERNEL && !CONFIG_SMP_ON_UP > * CONFIG_THUMB2_KERNEL && CONFIG_SMP_ON_UP > * !CONFIG_THUMB2_KERNEL && !CONFIG_SMP_ON_UP > * !CONFIG_THUMB2_KERNEL && CONFIG_SMP_ON_UP > > Signed-off-by: Dave Martin > --- > KernelVersion: 2.6.37-rc4 No need to mention but this patch changes lot of things around power management code. You said " Tested on: omap3 (Beagle xM A2)" What did you test ? Is it just boot test ? For sure just boot test is not enough for this patch and needs to test the PM. > > arch/arm/mach-omap2/pm.h |2 + > arch/arm/mach-omap2/pm34xx.c | 13 -- > arch/arm/mach-omap2/sleep34xx.S| 37 > +-- > arch/arm/mach-omap2/sram34xx.S | 34 +-- > - > arch/arm/plat-omap/include/plat/sram.h |1 + > arch/arm/plat-omap/sram.c | 10 +++- > 6 files changed, 80 insertions(+), 17 deletions(-) > > diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h > index 0d75bfd..c333bfd 100644 > --- a/arch/arm/mach-omap2/pm.h > +++ b/arch/arm/mach-omap2/pm.h > @@ -80,7 +80,9 @@ extern void save_secure_ram_context(u32 *addr); > extern void omap3_save_scratchpad_contents(void); > > extern unsigned int omap24xx_idle_loop_suspend_sz; > +extern char *const omap34xx_cpu_suspend_base; > extern unsigned int omap34xx_suspend_sz; > +extern char *const save_secure_ram_context_base; > extern unsigned int save_secure_ram_context_sz; > extern unsigned int omap24xx_cpu_suspend_sz; > extern unsigned int omap34xx_cpu_suspend_sz; > diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c > index 0ec8a04..93f0ee8 100644 > --- a/arch/arm/mach-omap2/pm34xx.c > +++ b/a
RE: [PATCH v2 2/3] ARM: omap4: Correct definition of do_wfi() forCONFIG_THUMB2_KERNEL
> -Original Message- > From: Dave Martin [mailto:dave.mar...@linaro.org] > Sent: Tuesday, December 07, 2010 10:21 PM > To: Santosh Shilimkar > Cc: linux-arm-ker...@lists.infradead.org; Tony Lindgren; linux- > o...@vger.kernel.org; linaro-dev@lists.linaro.org > Subject: Re: [PATCH v2 2/3] ARM: omap4: Correct definition of do_wfi() > forCONFIG_THUMB2_KERNEL > > Hi, > > On Tue, Dec 7, 2010 at 6:28 AM, Santosh Shilimkar > wrote: > > Dave, > >> -Original Message- > >> From: linaro-dev-boun...@lists.linaro.org [mailto:linaro-dev- > >> boun...@lists.linaro.org] On Behalf Of Dave Martin > >> Sent: Monday, December 06, 2010 11:06 PM > >> To: linux-arm-ker...@lists.infradead.org > >> Cc: Tony Lindgren; Dave Martin; linux-o...@vger.kernel.org; linaro- > >> d...@lists.linaro.org > >> Subject: [PATCH v2 2/3] ARM: omap4: Correct definition of do_wfi() > >> forCONFIG_THUMB2_KERNEL > >> > >> For the Thumb-2 case, the "wfi" mnemonic is used, since in this > >> case the tools will necessarily be new enough to support it. > >> > >> Signed-off-by: Dave Martin > >> --- > >> KernelVersion: 2.6.37-rc4 > > > > The choice of opcode instead of instruction here was not because > > of toolchain. The problem was it breaks multi-omap build where > > ARMv6 and ARMv7 are build together. > > > > For this reason I NAK this patch. > > You can't built a kernel for pre-v7 platforms with > CONFIG_THUMB2_KERNEL: the code can't run on those platforms because > they don't support Thumb-2. > That's better. > So anything inside #ifdef CONFIG_THUMB2_KERNEL can assume v7/Thumb-2 > capable (and hence reasonably new) tools. > > I'll follow up shortly with a patch to the generic ARM Kconfig to make > this explicit, so that ARCH_OMAP2 and THUMB2_KERNEL can't accidentally > be configured together. > sure ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
RE: [PATCH v2 2/3] ARM: omap4: Correct definition of do_wfi() forCONFIG_THUMB2_KERNEL
Dave, > -Original Message- > From: Santosh Shilimkar [mailto:santosh.shilim...@ti.com] > Sent: Wednesday, December 08, 2010 11:27 AM > To: Dave Martin > Cc: linux-arm-ker...@lists.infradead.org; Tony Lindgren; linux- > o...@vger.kernel.org; linaro-dev@lists.linaro.org > Subject: RE: [PATCH v2 2/3] ARM: omap4: Correct definition of do_wfi() > forCONFIG_THUMB2_KERNEL > > > -Original Message- > > From: Dave Martin [mailto:dave.mar...@linaro.org] > > Sent: Tuesday, December 07, 2010 10:21 PM > > To: Santosh Shilimkar > > Cc: linux-arm-ker...@lists.infradead.org; Tony Lindgren; linux- > > o...@vger.kernel.org; linaro-dev@lists.linaro.org > > Subject: Re: [PATCH v2 2/3] ARM: omap4: Correct definition of do_wfi() > > forCONFIG_THUMB2_KERNEL > > > > Hi, > > > > On Tue, Dec 7, 2010 at 6:28 AM, Santosh Shilimkar > > wrote: > > > Dave, [.] > > > So anything inside #ifdef CONFIG_THUMB2_KERNEL can assume v7/Thumb-2 > > capable (and hence reasonably new) tools. > > > > I'll follow up shortly with a patch to the generic ARM Kconfig to make > > this explicit, so that ARCH_OMAP2 and THUMB2_KERNEL can't accidentally > > be configured together. > > > sure When you are doing the changes can you please check if you could build the THUMB2 kernel with omap2plus_defconfig. I suspect the build will fail. Regards, Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
RE: [PATCH v2 2/3] ARM: omap4: Correct definition of do_wfi() forCONFIG_THUMB2_KERNEL
> -Original Message- > From: Dave Martin [mailto:dave.mar...@linaro.org] > Sent: Wednesday, December 08, 2010 4:11 PM > To: Santosh Shilimkar > Cc: linux-arm-ker...@lists.infradead.org; Tony Lindgren; linux- > o...@vger.kernel.org; linaro-dev@lists.linaro.org > Subject: Re: [PATCH v2 2/3] ARM: omap4: Correct definition of do_wfi() > forCONFIG_THUMB2_KERNEL > > On Wed, Dec 8, 2010 at 5:59 AM, Santosh Shilimkar > wrote: > > Dave, > > > >> -Original Message- > >> From: Santosh Shilimkar [mailto:santosh.shilim...@ti.com] > >> Sent: Wednesday, December 08, 2010 11:27 AM > >> To: Dave Martin > >> Cc: linux-arm-ker...@lists.infradead.org; Tony Lindgren; linux- > >> o...@vger.kernel.org; linaro-dev@lists.linaro.org > >> Subject: RE: [PATCH v2 2/3] ARM: omap4: Correct definition of do_wfi() > >> forCONFIG_THUMB2_KERNEL > >> > >> > -Original Message----- > >> > From: Dave Martin [mailto:dave.mar...@linaro.org] > >> > Sent: Tuesday, December 07, 2010 10:21 PM > >> > To: Santosh Shilimkar > >> > Cc: linux-arm-ker...@lists.infradead.org; Tony Lindgren; linux- > >> > o...@vger.kernel.org; linaro-dev@lists.linaro.org > >> > Subject: Re: [PATCH v2 2/3] ARM: omap4: Correct definition of > do_wfi() > >> > forCONFIG_THUMB2_KERNEL > >> > > >> > Hi, > >> > > >> > On Tue, Dec 7, 2010 at 6:28 AM, Santosh Shilimkar > >> > wrote: > >> > > Dave, > > > > [.] > >> > >> > So anything inside #ifdef CONFIG_THUMB2_KERNEL can assume v7/Thumb-2 > >> > capable (and hence reasonably new) tools. > >> > > >> > I'll follow up shortly with a patch to the generic ARM Kconfig to > make > >> > this explicit, so that ARCH_OMAP2 and THUMB2_KERNEL can't > accidentally > >> > be configured together. > >> > > >> sure > > > > When you are doing the changes can you please check if you could build > > the THUMB2 kernel with omap2plus_defconfig. I suspect the build will > > fail. > > With my Kconfig patch, kconfig won't let you turn on Thumb-2 in that > configuration: > > http://lists.arm.linux.org.uk/lurker/message/20101207.165737.0897658f.en.h > tml > > If you want to turn on Thumb-2, you must disable ARCH_OMAP2 first. If > my understanding is correct, this is the right behaviour. > > Ofcourse it will build with ARCH_OMAP2 disabled :) (ARMv6 dependency) In other words, I wanted to say that "omap2plus_defconfig" can't be used as is to build THUMB kernel binary. Thanks for conforming. ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
RE: [PATCH v2 2/3] ARM: omap4: Correct definition of do_wfi() forCONFIG_THUMB2_KERNEL
> -Original Message- > From: Dave Martin [mailto:dave.mar...@linaro.org] > Sent: Wednesday, December 08, 2010 4:35 PM > To: Santosh Shilimkar > Cc: Tony Lindgren; linux-o...@vger.kernel.org; linaro- > d...@lists.linaro.org; linux-arm-ker...@lists.infradead.org > Subject: Re: [PATCH v2 2/3] ARM: omap4: Correct definition of do_wfi() > forCONFIG_THUMB2_KERNEL > > On Wed, Dec 8, 2010 at 10:45 AM, Santosh Shilimkar > wrote: > >> -Original Message- > >> From: Dave Martin [mailto:dave.mar...@linaro.org] > >> Sent: Wednesday, December 08, 2010 4:11 PM > >> To: Santosh Shilimkar > >> Cc: linux-arm-ker...@lists.infradead.org; Tony Lindgren; linux- > >> o...@vger.kernel.org; linaro-dev@lists.linaro.org > >> Subject: Re: [PATCH v2 2/3] ARM: omap4: Correct definition of do_wfi() > >> forCONFIG_THUMB2_KERNEL > >> > >> On Wed, Dec 8, 2010 at 5:59 AM, Santosh Shilimkar > >> wrote: > >> > Dave, > >> > > >> >> -Original Message- > >> >> From: Santosh Shilimkar [mailto:santosh.shilim...@ti.com] > >> >> Sent: Wednesday, December 08, 2010 11:27 AM > >> >> To: Dave Martin > >> >> Cc: linux-arm-ker...@lists.infradead.org; Tony Lindgren; linux- > >> >> o...@vger.kernel.org; linaro-dev@lists.linaro.org > >> >> Subject: RE: [PATCH v2 2/3] ARM: omap4: Correct definition of > > do_wfi() > >> >> forCONFIG_THUMB2_KERNEL > >> >> > >> >> > -Original Message- > >> >> > From: Dave Martin [mailto:dave.mar...@linaro.org] > >> >> > Sent: Tuesday, December 07, 2010 10:21 PM > >> >> > To: Santosh Shilimkar > >> >> > Cc: linux-arm-ker...@lists.infradead.org; Tony Lindgren; linux- > >> >> > o...@vger.kernel.org; linaro-dev@lists.linaro.org > >> >> > Subject: Re: [PATCH v2 2/3] ARM: omap4: Correct definition of > >> do_wfi() > >> >> > forCONFIG_THUMB2_KERNEL > >> >> > > >> >> > Hi, > >> >> > > >> >> > On Tue, Dec 7, 2010 at 6:28 AM, Santosh Shilimkar > >> >> > wrote: > >> >> > > Dave, > >> > > >> > [.] > >> >> > >> >> > So anything inside #ifdef CONFIG_THUMB2_KERNEL can assume > > v7/Thumb-2 > >> >> > capable (and hence reasonably new) tools. > >> >> > > >> >> > I'll follow up shortly with a patch to the generic ARM Kconfig to > >> make > >> >> > this explicit, so that ARCH_OMAP2 and THUMB2_KERNEL can't > >> accidentally > >> >> > be configured together. > >> >> > > >> >> sure > >> > > >> > When you are doing the changes can you please check if you could > build > >> > the THUMB2 kernel with omap2plus_defconfig. I suspect the build will > >> > fail. > >> > >> With my Kconfig patch, kconfig won't let you turn on Thumb-2 in that > >> configuration: > >> > >> > > > http://lists.arm.linux.org.uk/lurker/message/20101207.165737.0897658f.en.h > >> tml > >> > >> If you want to turn on Thumb-2, you must disable ARCH_OMAP2 first. If > >> my understanding is correct, this is the right behaviour. > >> > >> > > Ofcourse it will build with ARCH_OMAP2 disabled :) (ARMv6 dependency) > > In other words, I wanted to say that "omap2plus_defconfig" can't > > be used as is to build THUMB kernel binary. > > Well, yes, that's another way of looking at it. Anyway, I think this > is the intended result -- is it OK for you? > Should be ok considering you can't do much with it. But I let Tony comment on it. Regards, Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
RE: [PM] cpufreq test
> -Original Message- > From: linaro-dev-boun...@lists.linaro.org [mailto:linaro-dev- > boun...@lists.linaro.org] On Behalf Of Vincent Guittot > Sent: Monday, February 07, 2011 5:34 PM > To: Vishwanath Sripathy > Cc: linaro-dev@lists.linaro.org > Subject: Re: [PM] cpufreq test > > On 7 February 2011 12:55, Vishwanath Sripathy > wrote: > > Vincent, > > > > On Mon, Feb 7, 2011 at 3:55 PM, Vincent Guittot > > wrote: > >> Hi vishwa, > >> > >> I have passed cpufreq-bench on my platform. The results below > have > >> been reached with a sampling rate of 20ms and a sampling_down > factor > >> set to 10. > >> > >> I have a question about the sampling_down feature : The ondemand > delay > >> value is calculated before calling dbs_check_cpu, but > dbs_check_cpu > >> can modify rate_mult. This implies that if the rate_mult is set > to 1 > >> in dbs_check_cpu because we set something else than the max > frequency, > >> the next sampling period will be > sampling_rate*sampling_down_factor > >> (the one used for max freq) but the frequency can be the lowest > one. > >> We should rather calculate the delay after dbs_check_cpu or i > miss > >> something ? > > Good point. Yes, with current implementation, change in rate_mult > will > > have effect only after next do_dbs_timer is called and the point > > raised by you is perfectly valid. Why don't you post this query to > > cpu_freq mailing list? > > > > ok, i'm going to send it > May be just send the patch to fix it. Regards, Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
RE: linaro-omap kernel 1003.6 boot failure
> -Original Message- > From: linaro-dev-boun...@lists.linaro.org [mailto:linaro-dev- > boun...@lists.linaro.org] On Behalf Of Turgis, Frederic > Sent: Monday, February 21, 2011 11:20 PM > To: linaro-dev@lists.linaro.org > Subject: linaro-omap kernel 1003.6 boot failure > > Hi, (writing from my work address as frederic.tur...@linaro.org lost > password is still not reset ;-) ) > > To perform some systemtap checking, I need kernel package with debug > symbols, which is available in 1003.6 version from > http://ddebs.ubuntu.com/pool/universe/l/linux-linaro-omap/ > So I did some apt-get update, install linux-image-2.6.37-1003- > linaro-omap followed by flash-kernel 2.6.37-1003-linaro-omap. > Boot fails at some point (before setting up omap_hsuart in an OK log > from 2.6.37-1002) > > Any known issue or thing I am doing wrong ? > Try this series and see if you can locate the offending module ? http://www.mail-archive.com/linux-omap@vger.kernel.org/msg45021.html > > U-Boot 2010.12 (Jan 27 2011 - 17:59:04) > > CPU : OMAP4430 > Board: OMAP4 Panda > I2C: ready > DRAM: 1 GiB > MMC: OMAP SD/MMC: 0 > Using default environment > > In:serial > Out: serial > Err: serial > Hit any key to stop autoboot: 0 > reading boot.scr > > 381 bytes read > Running bootscript from mmc0 ... > ## Executing script at 8200 > reading uImage > > 4000628 bytes read > reading uInitrd > > 4154752 bytes read > ## Booting kernel from Legacy Image at 8020 ... >Image Name: Ubuntu Kernel >Image Type: ARM Linux Kernel Image (uncompressed) >Data Size:4000564 Bytes = 3.8 MiB >Load Address: 80008000 >Entry Point: 80008000 >Verifying Checksum ... OK > ## Loading init Ramdisk from Legacy Image at 8160 ... >Image Name: Ubuntu Initrd >Image Type: ARM Linux RAMDisk Image (uncompressed) >Data Size:4154688 Bytes = 4 MiB >Load Address: >Entry Point: >Verifying Checksum ... OK >Loading Kernel Image ... OK > OK > > Starting kernel ... > > Uncompressing Linux... done, booting the kernel. > [0.00] Initializing cgroup subsys cpuset > [0.00] Initializing cgroup subsys cpu > [0.00] Linux version 2.6.37-1003-linaro-omap (buildd@gourd) > (gcc version 4.5.2 (Ubuntu/Linaro 4.5.2-2ubuntu2) ) #6-Ubuntu SMP > Fri Feb 11 18:49:17 UTC 2011 (Ubuntu 2.6.37-1003.6-linaro-omap > 2.6.37) > [0.00] CPU: ARMv7 Processor [411fc092] revision 2 (ARMv7), > cr=10c53c7f > [0.00] CPU: VIPT nonaliasing data cache, VIPT aliasing > instruction cache > [0.00] Machine: OMAP4 Panda board > [0.00] bootconsole [earlycon0] enabled > [0.00] Reserving 33554432 bytes SDRAM for VRAM > [0.00] Memory policy: ECC disabled, Data cache writealloc > [0.00] OMAP4430 ES2.0 > [0.00] SRAM: Mapped pa 0x4030 to va 0xfe40 size: > 0xe000 > [0.00] FIXME: omap44xx_sram_init not implemented > [0.00] PERCPU: Embedded 7 pages/cpu @c0c5b000 s7680 r8192 > d12800 u32768 > [0.00] Built 1 zonelists in Zone order, mobility grouping > on. Total pages: 109410 > [0.00] Kernel command line: console=tty0 > console=ttyO2,115200n8 root=UUID=dfc5a269-1ede-4174-b52a- > 02a1297a52f7 rootwait ro earlyprintk fixrtc nocompcache vram=32M > omapfb.vram=0:8M mem=463M ip=none > [0.00] PID hash table entries: 2048 (order: 1, 8192 bytes) > [0.00] Dentry cache hash table entries: 65536 (order: 6, > 262144 bytes) > [0.00] Inode-cache hash table entries: 32768 (order: 5, > 131072 bytes) > [0.00] allocated 2370560 bytes of page_cgroup > [0.00] please try 'cgroup_disable=memory' option if you > don't want memory cgroups > [0.00] Memory: 430MB 1MB = 431MB total > [0.00] Memory: 421860k/421860k available, 52252k reserved, > 0K highmem > [0.00] Virtual kernel memory layout: > [0.00] vector : 0x - 0x1000 ( 4 kB) > [0.00] fixmap : 0xfff0 - 0xfffe ( 896 kB) > [0.00] DMA : 0xffc0 - 0xffe0 ( 2 MB) > [0.00] vmalloc : 0xdd00 - 0xf800 ( 432 MB) > [0.00] lowmem : 0xc000 - 0xdcf0 ( 463 MB) > [0.00] modules : 0xbf00 - 0xc000 ( 16 MB) > [0.00] .init : 0xc0008000 - 0xc0053000 ( 300 kB) > [0.00] .text : 0xc0053000 - 0xc078a32c (7389 kB) > [0.00] .data : 0xc078c000 - 0xc07f04e0 ( 402 kB) > [0.00] SLUB: Genslabs=13, HWalign=32, Order=0-3, > MinObjects=0, CPUs=2, Nodes=1 > [0.00] Hierarchical RCU implementation. > [0.00] RCU-based detection of stalled CPUs is disabled. > [0.00] NR_IRQS:402 > [0.00] clockdomain: l3_dma_clkdm: clkdm_clear_all_wkdeps: > not yet implemented > [0.00] clockdomain: emu_sys_clkdm: OMAP4 wakeup/sleep > dependency support: not yet implemented > [0.00] clockdomain: emu_sys_clkdm: clkdm_cl
RE: [PATCH v5 REPOST 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
Dave, > -Original Message- > From: Dave Martin [mailto:dave.mar...@linaro.org] > Sent: Friday, March 04, 2011 11:05 PM > To: linux-arm-ker...@lists.infradead.org > Cc: patc...@linaro.org; Tony Lindgren; Santosh Shilimkar; Jean > Pihet; Kevin Hilman; linux-o...@vger.kernel.org; Nicolas Pitre; > linaro-dev@lists.linaro.org > Subject: Re: [PATCH v5 REPOST 0/5] ARM: omap[34]: Thumb-2 > compatibility fixes > > On Fri, Mar 4, 2011 at 3:33 PM, Dave Martin > wrote: > > This set of patches, along with some other patches under > > discussion on alkml, should enable omap3 and omap4 kernels to be > > built with CONFIG_THUMB2_KERNEL. > > > > This patch set builds on recent cleanup done by the omap > > maintainers. > > > > At least some of this code definitely works, most features have > > been tested successfully. Further testing, especially of Thumb-2 > > behaviour, is still welcome. > > > > In particular, I still have no Ack/Tested-by on the following > > patches, so it would be great if anyone who has an opportunity > > to do a final review / re-test can do so. > > > > * ARM: omap3: Convert END() to ENDPROC() for correct linkage with > CONFIG_THUMB2_KERNEL > > * ARM: omap3: Thumb-2 compatibility for sram34xx.S > > * ARM: omap3: Thumb-2 compatibility for sleep34xx.S > > Following up on this, it looks like I have some locally-recorded > acks > which didn't make it into my posting ... > > The one patch I currently have no feedback on is this one: > > * ARM: omap4: Provide do_wfi() for Thumb-2 > You can drop this one since do_wfi() won't be needed anymore after my recent series. http://www.spinics.net/lists/linux-omap/msg46495.html ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
RE: [PATCH v5 REPOST 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
> -Original Message- > From: Kevin Hilman [mailto:khil...@ti.com] > Sent: Tuesday, March 08, 2011 3:05 AM > To: Santosh Shilimkar > Cc: Dave Martin; linux-arm-ker...@lists.infradead.org; > patc...@linaro.org; Tony Lindgren; Jean Pihet-XID; linux- > o...@vger.kernel.org; Nicolas Pitre; linaro-dev@lists.linaro.org > Subject: Re: [PATCH v5 REPOST 0/5] ARM: omap[34]: Thumb-2 > compatibility fixes > > Santosh Shilimkar writes: > > > Dave, > >> -Original Message- > >> From: Dave Martin [mailto:dave.mar...@linaro.org] > >> Sent: Friday, March 04, 2011 11:05 PM > >> To: linux-arm-ker...@lists.infradead.org > >> Cc: patc...@linaro.org; Tony Lindgren; Santosh Shilimkar; Jean > >> Pihet; Kevin Hilman; linux-o...@vger.kernel.org; Nicolas Pitre; > >> linaro-dev@lists.linaro.org > >> Subject: Re: [PATCH v5 REPOST 0/5] ARM: omap[34]: Thumb-2 > >> compatibility fixes > >> > >> On Fri, Mar 4, 2011 at 3:33 PM, Dave Martin > > >> wrote: > >> > This set of patches, along with some other patches under > >> > discussion on alkml, should enable omap3 and omap4 kernels to > be > >> > built with CONFIG_THUMB2_KERNEL. > >> > > >> > This patch set builds on recent cleanup done by the omap > >> > maintainers. > >> > > >> > At least some of this code definitely works, most features have > >> > been tested successfully. Further testing, especially of > Thumb-2 > >> > behaviour, is still welcome. > >> > > >> > In particular, I still have no Ack/Tested-by on the following > >> > patches, so it would be great if anyone who has an opportunity > >> > to do a final review / re-test can do so. > >> > > >> > * ARM: omap3: Convert END() to ENDPROC() for correct linkage > with > >> CONFIG_THUMB2_KERNEL > >> > * ARM: omap3: Thumb-2 compatibility for sram34xx.S > >> > * ARM: omap3: Thumb-2 compatibility for sleep34xx.S > >> > >> Following up on this, it looks like I have some locally-recorded > >> acks > >> which didn't make it into my posting ... > >> > >> The one patch I currently have no feedback on is this one: > >> > >> * ARM: omap4: Provide do_wfi() for Thumb-2 > >> > > You can drop this one since do_wfi() won't be needed > > anymore after my recent series. > > > > http://www.spinics.net/lists/linux-omap/msg46495.html > > Santosh, > > I'm planning to queue/merge Dave's series as-is for 2.6.39, since it > has > already received significan review & testing. > > When you update your OMAP4 series, feel free to drop it as needed. > Fine with me. Regards, Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [Config] Select ARM_ERRATA_753970 for Pandaboard
On 5/6/2011 2:22 PM, Paolo Pisati wrote: flag@omap:~$ dmesg | grep -A 1 L310 [0.219024] L310 cache controller enabled [0.219055] l2x0: 16 ways, CACHE_ID 0x41c4, AUX_CTRL 0x7e47, Cache size: 1048576 B ^^ according to [1], that means Panda has a r3p0 L310 L2 controller. Something wrong in ID decoding. The actual PL310 version used on OMAP4430 ES2.x is r2p0 and hence errata 753970 is NA for OMAP4430 PANDA/BLAZE/SDP. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [Config] Select ARM_ERRATA_753970 for Pandaboard
On 5/6/2011 7:03 PM, Paolo Pisati wrote: On 05/06/2011 03:12 PM, Santosh Shilimkar wrote: Something wrong in ID decoding. The actual PL310 version used on OMAP4430 ES2.x is r2p0 and hence errata 753970 is NA for OMAP4430 PANDA/BLAZE/SDP. weird since it's just a pci read: arm/mm/cache-l2x0.c::l2x0_init() ... cache_id = readl_relaxed(l2x0_base + L2X0_CACHE_ID); ... printk(KERN_INFO "l2x0: %d ways, CACHE_ID 0x%08x, AUX_CTRL 0x%08x, Cache size: %d B\n", ways, cache_id, aux, l2x0_size); wrong id on the datasheet? Not sure. Will confirm with TI hardware team but am quite certain about the PL310 version used on OMAP4430. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [Config] Select ARM_ERRATA_753970 for Pandaboard
On 5/6/2011 7:57 PM, Santosh Shilimkar wrote: On 5/6/2011 7:03 PM, Paolo Pisati wrote: On 05/06/2011 03:12 PM, Santosh Shilimkar wrote: [...] wrong id on the datasheet? Not sure. Will confirm with TI hardware team but am quite certain about the PL310 version used on OMAP4430. Got confirmation form hardware team that OMAP4430 PL310 version is r2p0 and hence the errata is NA. Regards Santosh ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev