* [PATCHv5 01/16] arm: arch_timer: balance device_node refcounting
From: Catalin Marinas @ 2013-01-31 15:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1359634539-9580-2-git-send-email-mark.rutland@arm.com>
On Thu, Jan 31, 2013 at 12:15:24PM +0000, Mark Rutland wrote:
> When we get the device_node for the arch timer, it's refcount is
> automatically incremented in of_find_matching_node, but it is
> never decremented.
>
> This patch decrements the refcount on the node after we're finished
> using it.
>
> Reported-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* [PATCHv5 04/16] arm: arch_timer: standardise counter reading
From: Catalin Marinas @ 2013-01-31 15:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1359634539-9580-5-git-send-email-mark.rutland@arm.com>
On Thu, Jan 31, 2013 at 12:15:27PM +0000, Mark Rutland wrote:
> We're currently inconsistent with respect to our accesses to the
> physical and virtual counters, mixing and matching the two.
>
> This patch introduces and uses a function pointer for accessing the
> correct counter based on whether we're using physical or virtual
> interrupts. All current accesses to the counter accessors are redirected
> through it.
>
> When the driver is moved out to drivers/clocksource, there's the
> possibility that code called before the timer code is initialised will
> attempt to call arch_timer_read_counter (e.g. sched_clock for AArch64).
> To avoid having to have to check whether the timer has been initialised
> either in arch_timer_read_counter or one of it's callers, a default
> implementation is assigned that simply returns 0.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* IPC SHM alignment on ARMv7
From: Alexander Kartashov @ 2013-01-31 15:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130131134747.GY23505@n2100.arm.linux.org.uk>
On 01/31/2013 05:47 PM, Russell King - ARM Linux wrote:
> Err, no. Try again
OK, let me be more specific. The following code is a simplified
version of the test in the CRIU test suite that checks IPC SHM
checkpoint/restoration work correctly:
#include <stdio.h>
#include <sys/ipc.h>
int main()
{
int loop;
int shmid = shmget(0x94646337, 40960, IPC_CREAT);
if (shmid < 0) {
perror("Failed to get a SHM descriptor");
return 1;
}
void *addr1, *addr2;
addr1 = shmat(shmid, 0, 0);
if (addr1 == (void*)-1) {
perror("Failed to attach the SHM segment");
return 2;
}
printf("Attached to %p\n", addr1);
*((int*)addr1) = 1;
shmdt(addr1);
addr2 = shmat(shmid, addr1, 0);
if (addr2 == (void*)-1) {
perror("Failed to re-attach the SHM");
printf("The SHM segment was formerly attached to %p\n", addr1);
return 3;
}
if (addr2 != addr1) {
printf("The SHM segment address changed: was %p, now %p\n",
addr1, addr2);
return 4;
}
return 0;
}
I'm running this code in the Linux 3.7.5:
root at crtest:~/ipc-fail# cat /proc/version
Linux version 3.7.5 (alex at alex-pc) (gcc version 4.6.3 20120201
(prerelease) (crosstool-NG linaro-1.13.1-2012.02-20120222 - Linaro GCC
2012.02) ) #1 SMP Thu Jan 31 19:01:37 MSK 2013
I'm running the kernel in the QEMU model of the board RM Versatile
Express for Cortex-A9.
I use the config vexpress_defconfig to compile the kernel.
The test usually outputs something like:
root at crtest:~/ipc-fail# ./ipc-fail
Attached to 0x76e22000
Failed to re-attach the SHM: Invalid argument
The SHM segment was formely attached to 0x76e22000
root at crtest:~/ipc-fail# ./ipc-fail
Attached to 0x76df6000
Failed to re-attach the SHM: Invalid argument
The SHM segment was formely attached to 0x76df6000
However it sometimes succeeds:
root@crtest:~/ipc-fail# ./ipc-fail
Attached to 0x76e94000
As you can see the test fails when the address returned by
the function shmat() isn't SHMLBA-aligned.
I think that this is caused by the fact that the kernel function do_shmat()
requires the argument shmaddr to be SHMLBA-aligned:
[ipc/shm.c]
[...]
958 long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong
*raddr,
959 unsigned long shmlba)
960 {
[...]
974
975 err = -EINVAL;
976 if (shmid < 0)
977 goto out;
978 else if ((addr = (ulong)shmaddr)) {
979 if (addr & (shmlba - 1)) {
980 if (shmflg & SHM_RND)
981 addr &= ~(shmlba - 1); /* round down */
982 else
983 #ifndef __ARCH_FORCE_SHMLBA
984 if (addr & ~PAGE_MASK)
985 #endif
986 goto out;
However, it can't guarantee that its returned address is SHMLBA-aligned
because the function arch_get_unmapped_area():
[arch/arm/mm/mmap.c]
[...]
66 unsigned long
67 arch_get_unmapped_area(struct file *filp, unsigned long addr,
68 unsigned long len, unsigned long pgoff, unsigned long flags)
69 {
70 struct mm_struct *mm = current->mm;
71 struct vm_area_struct *vma;
72 unsigned long start_addr;
73 int do_align = 0;
74 int aliasing = cache_is_vipt_aliasing();
75
76 /*
77 * We only need to do colour alignment if either the I or D
78 * caches alias.
79 */
80 if (aliasing)
81 do_align = filp || (flags & MAP_SHARED);
[...]
114 full_search:
115 if (do_align)
116 addr = COLOUR_ALIGN(addr, pgoff);
117 else
118 addr = PAGE_ALIGN(addr);
119
120 for (vma = find_vma(mm, addr); ; vma = vma->vm_next) {
121 /* At this point: (!vma || addr < vma->vm_end). */
122 if (TASK_SIZE - len < addr) {
123 /*
124 * Start a new search - just in case we missed
125 * some holes.
126 */
127 if (start_addr != TASK_UNMAPPED_BASE) {
128 start_addr = addr = TASK_UNMAPPED_BASE;
129 mm->cached_hole_size = 0;
130 goto full_search;
131 }
132 return -ENOMEM;
133 }
134 if (!vma || addr + len <= vma->vm_start) {
135 /*
136 * Remember the place where we stopped the search:
137 */
138 mm->free_area_cache = addr + len;
139 return addr;
140 }
141 if (addr + mm->cached_hole_size < vma->vm_start)
142 mm->cached_hole_size = vma->vm_start - addr;
143 addr = vma->vm_end;
144 if (do_align)
145 addr = COLOUR_ALIGN(addr, pgoff);
146 }
147 }
aligns the returned address on a SHMLBA-boundary only if I or D caches alias
as the comment reads:
76 /*
77 * We only need to do colour alignment if either the I or D
78 * caches alias.
79 */
that isn't true for ARMv7.
So the question is whether it's possible to align a SHM segement
on a SHMLBA boundary unconditionally.
--
Sincerely yours,
Alexander Kartashov
Intern
Core team
www.parallels.com
Skype: aleksandr.kartashov
Email: alekskartashov at parallels.com
^ permalink raw reply
* [PATCHv5 07/16] arm: arch_timer: add isbs to register accessors
From: Catalin Marinas @ 2013-01-31 15:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1359634539-9580-8-git-send-email-mark.rutland@arm.com>
On Thu, Jan 31, 2013 at 12:15:30PM +0000, Mark Rutland wrote:
> Without the isbs in arch_timer_get_cnt{p,v}ct the cpu may speculate
> reads and return stale values. This could be bad for code sensitive to
> changes in expected deltas between calls (e.g. the delay loop).
>
> Without isbs in arch_timer_reg_write the processor may reorder
> instructions around enabling/disabling of the timer or writing the
> compare value, which we probably don't want.
>
> This patch adds isbs to prevent those issues.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* [PATCHv5 09/16] arm: arch_timer: add arch_counter_set_user_access
From: Catalin Marinas @ 2013-01-31 15:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1359634539-9580-10-git-send-email-mark.rutland@arm.com>
On Thu, Jan 31, 2013 at 12:15:32PM +0000, Mark Rutland wrote:
> Several bits in CNTKCTL reset to 0, including PL0VTEN. For architectures
> using the generic timer which wish to have a fast gettimeofday vDSO
> implementation, these bits must be set to 1 by the kernel. For
> architectures without a vDSO, it's best to leave the bits set to 0 for
> now to ensure that if and when support is added, it's implemented sanely
> architecture wide.
>
> As the bootloader might set PL0VTEN to a value that doesn't correspond
> to that which the kernel prefers, we must explicitly set it to the
> architecture port's preferred value.
>
> This patch adds arch_counter_set_user_access, which sets the PL0 access
> permissions to that required by the architecture. For arch/arm, this
> currently means disabling all userspace access.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* [PATCH v2 19/27] pci: PCIe driver for Marvell Armada 370/XP systems
From: Russell King - ARM Linux @ 2013-01-31 15:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130131162237.79de29c4@skate>
On Thu, Jan 31, 2013 at 04:22:37PM +0100, Thomas Petazzoni wrote:
> Dear Russell King - ARM Linux,
>
> On Thu, 31 Jan 2013 15:08:01 +0000, Russell King - ARM Linux wrote:
> > If that's what your bridge requires, then we need to detect it via
> > its vendor and device IDs and only apply this fixup to those bridges
> > which require a 64K alignment.
> >
> > So, the IDs are vendor:device = 0x11ab:0x1092 ? And let me get this
> > straight, it _is_ a specific requirement for this particular bridge
> > P2P bridge?
>
> The vendor/device ID will change. This P2P bridge is emulated. However,
> I'm not sure why you need to know the vendor:device ID to make the
> fixup specific.
>
> The fixup is already specific to those bridges, since I am just setting
> pci_sys_data->win_align_io to 64K for the particular buses that are
> downstream the problematic bridges. So it doesn't affect any other bus
> on the system, and therefore I don't think this fixup needs to be made
> specific to a given vendor:device, no?
The pci_sys_data is not specific to one bus. It's specific from the
root bus downwards, and is shared by all child busses.
The problem is if you have some card or a conventional P2P bridge which
has 4K windows. If you merely set the alignment to 64K for all bridges,
then all bridges get this treatment whether or not they need it. That's
what I'm trying to avoid.
Take, for instance, a cardbus bridge (remember, there are PCI cards which
can be plugged in to give you a cardbus slot.) I have a device here which
can be plugged into a cardbus slot which has not just one P2P bridge but
two, and a bunch of downsteam devices, including VGA, ethernet, USB, PS/2
etc. (Okay, Linux doesn't support this hardware because of crappy X86
stuff, despite the fact Windows cope with it just fine.)
There have been cards in the past which have had P2P bridges on them as
well.
So, simply believing that the only P2P bridges in the system will be
those on the physical board is a mistake.
^ permalink raw reply
* [PATCH 1/2] ARM: use read_cpuid_id() instead of read_cpuid(CPUID_ID)
From: Uwe Kleine-König @ 2013-01-31 15:36 UTC (permalink / raw)
To: linux-arm-kernel
Both calls are identical currently. This patch prepares to deprecate
read_cpuid on machines without cp15.
Also move an unconditional usage of read_cpuid_cachetype to a more local
scope as read_cpuid_cachetype uses read_cpuid, too.
Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
arch/arm/kernel/setup.c | 2 +-
arch/arm/kernel/smp_scu.c | 2 +-
arch/arm/mach-omap2/id.c | 4 ++--
arch/arm/mach-omap2/omap-smp.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 3f6cbb2..1cc9e17 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -291,10 +291,10 @@ static int cpu_has_aliasing_icache(unsigned int arch)
static void __init cacheid_init(void)
{
- unsigned int cachetype = read_cpuid_cachetype();
unsigned int arch = cpu_architecture();
if (arch >= CPU_ARCH_ARMv6) {
+ unsigned int cachetype = read_cpuid_cachetype();
if ((cachetype & (7 << 29)) == 4 << 29) {
/* ARMv7 register format */
arch = CPU_ARCH_ARMv7;
diff --git a/arch/arm/kernel/smp_scu.c b/arch/arm/kernel/smp_scu.c
index b9f015e..743a3bf 100644
--- a/arch/arm/kernel/smp_scu.c
+++ b/arch/arm/kernel/smp_scu.c
@@ -41,7 +41,7 @@ void scu_enable(void __iomem *scu_base)
#ifdef CONFIG_ARM_ERRATA_764369
/* Cortex-A9 only */
- if ((read_cpuid(CPUID_ID) & 0xff0ffff0) == 0x410fc090) {
+ if ((read_cpuid_id() & 0xff0ffff0) == 0x410fc090) {
scu_ctrl = __raw_readl(scu_base + 0x30);
if (!(scu_ctrl & 1))
__raw_writel(scu_ctrl | 0x1, scu_base + 0x30);
diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 45cc7ed4..1377c36 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -300,7 +300,7 @@ void __init omap3xxx_check_revision(void)
* If the processor type is Cortex-A8 and the revision is 0x0
* it means its Cortex r0p0 which is 3430 ES1.0.
*/
- cpuid = read_cpuid(CPUID_ID);
+ cpuid = read_cpuid_id();
if ((((cpuid >> 4) & 0xfff) == 0xc08) && ((cpuid & 0xf) == 0x0)) {
omap_revision = OMAP3430_REV_ES1_0;
cpu_rev = "1.0";
@@ -450,7 +450,7 @@ void __init omap4xxx_check_revision(void)
* Use ARM register to detect the correct ES version
*/
if (!rev && (hawkeye != 0xb94e) && (hawkeye != 0xb975)) {
- idcode = read_cpuid(CPUID_ID);
+ idcode = read_cpuid_id();
rev = (idcode & 0xf) - 1;
}
diff --git a/arch/arm/mach-omap2/omap-smp.c b/arch/arm/mach-omap2/omap-smp.c
index cd42d92..707098e 100644
--- a/arch/arm/mach-omap2/omap-smp.c
+++ b/arch/arm/mach-omap2/omap-smp.c
@@ -209,7 +209,7 @@ static void __init omap4_smp_init_cpus(void)
unsigned int i = 0, ncores = 1, cpu_id;
/* Use ARM cpuid check here, as SoC detection will not work so early */
- cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
+ cpu_id = read_cpuid_id() & CPU_MASK;
if (cpu_id == CPU_CORTEX_A9) {
/*
* Currently we can't call ioremap here because
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/2] ARM: stub out read_cpuid and read_cpuid_ext for CPU_CP15=n
From: Uwe Kleine-König @ 2013-01-31 15:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1359646587-1788-1-git-send-email-u.kleine-koenig@pengutronix.de>
Traditionally for !CPU_CP15 read_cpuid and read_cpuid_ext returned the
processor id independent of the parameter passed in. This is wrong of
course but theoretically this doesn't harm because it's only called on
machines having a cp15.
Instead return 0 unconditionally which might make unused code paths be
better optimizable and so smaller and warn about unexpected usage.
Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
arch/arm/include/asm/cputype.h | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
index a59dcb5..574269e 100644
--- a/arch/arm/include/asm/cputype.h
+++ b/arch/arm/include/asm/cputype.h
@@ -50,6 +50,7 @@ extern unsigned int processor_id;
: "cc"); \
__val; \
})
+
#define read_cpuid_ext(ext_reg) \
({ \
unsigned int __val; \
@@ -59,12 +60,25 @@ extern unsigned int processor_id;
: "cc"); \
__val; \
})
-#else
-#define read_cpuid(reg) (processor_id)
-#define read_cpuid_ext(reg) 0
-#endif
+
+#else /* ifdef CONFIG_CPU_CP15 */
/*
+ * read_cpuid and read_cpuid_ext should only ever be called on machines that
+ * have cp15 so warn on other usages.
+ */
+#define read_cpuid(reg) \
+ ({ \
+ WARN_ON_ONCE(1); \
+ 0; \
+ })
+
+#define read_cpuid_ext(reg) read_cpuid(reg)
+
+#endif /* ifdef CONFIG_CPU_CP15 / else */
+
+#ifdef CONFIG_CPU_CP15
+/*
* The CPU ID never changes at run time, so we might as well tell the
* compiler that it's constant. Use this function to read the CPU ID
* rather than directly reading processor_id or read_cpuid() directly.
@@ -74,6 +88,15 @@ static inline unsigned int __attribute_const__ read_cpuid_id(void)
return read_cpuid(CPUID_ID);
}
+#else /* ifdef CONFIG_CPU_CP15 */
+
+static inline unsigned int __attribute_const__ read_cpuid_id(void)
+{
+ return processor_id;
+}
+
+#endif /* ifdef CONFIG_CPU_CP15 / else */
+
static inline unsigned int __attribute_const__ read_cpuid_cachetype(void)
{
return read_cpuid(CPUID_CACHETYPE);
--
1.7.10.4
^ permalink raw reply related
* [PATCHv5 11/16] arm64: arm_generic: prevent reading stale time
From: Catalin Marinas @ 2013-01-31 15:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1359634539-9580-12-git-send-email-mark.rutland@arm.com>
On Thu, Jan 31, 2013 at 12:15:34PM +0000, Mark Rutland wrote:
> Currently arch_counter_get_cnt{p,v}ct can be speculated, allowing for
> stale time values to be read. This could be problematic for the delay
> loop and other sensitive functions, as the time delta could jump around
> unexpectedly.
>
> This patch adds isbs to arch_counter_get_cnt{p,v}ct, preventing this
> possibility.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* [PATCHv5 14/16] ARM: arch_timers: switch to physical timers if HYP mode is available
From: Catalin Marinas @ 2013-01-31 15:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1359634539-9580-15-git-send-email-mark.rutland@arm.com>
On Thu, Jan 31, 2013 at 12:15:37PM +0000, Mark Rutland wrote:
> From: Marc Zyngier <Marc.Zyngier@arm.com>
>
> If we're booted in HYP mode, it is possible that we'll run some
> kind of virtualized environment. In this case, it is a better to
> switch to the physical timers, and leave the virtual timers to
> guests.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* [PATCH] ARM: OMAP2+: Get rid of custom OMAP_32K_TIMER_HZ
From: Arnd Bergmann @ 2013-01-31 15:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1359646320-9086-1-git-send-email-santosh.shilimkar@ti.com>
On Thursday 31 January 2013 21:02:00 Santosh Shilimkar wrote:
> The timekeeping doesn't depend on HZ value in presence of fine grained
> clocksource and hence there should not be any time drift because of HZ
> value which was chosen to be divisor of 32768.
>
> OMAP has been using HZ = 128 value to avoid any time drift issues
> because of 32768 HZ clock. But with various measurements performed
> with HZ = 100, no time drift is observed and it also proves the
> point about HZ not having impact on time keeping on OMAP.
>
> Very informative thread on this topic is here:
> https://lkml.org/lkml/2013/1/29/435
>
> Special thanks to John Stulz, Arnd Bergmann and Russell King for their
> valuable suggestions.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Tony Lindgren <tony@atomide.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Thanks so much for looking into this!
^ permalink raw reply
* [PATCHv5 16/16] clocksource: arch_timer: use virtual counters
From: Catalin Marinas @ 2013-01-31 15:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1359634539-9580-17-git-send-email-mark.rutland@arm.com>
On Thu, Jan 31, 2013 at 12:15:39PM +0000, Mark Rutland wrote:
> Switching between reading the virtual or physical counters is
> problematic, as some core code wants a view of time before we're fully
> set up. Using a function pointer and switching the source after the
> first read can make time appear to go backwards, and having a check in
> the read function is an unfortunate block on what we want to be a fast
> path.
>
> Instead, this patch makes us always use the virtual counters. If we're a
> guest, or don't have hyp mode, we'll use the virtual timers, and as such
> don't care about CNTVOFF as long as it doesn't change in such a way as
> to make time travel backwards. If we do have hyp mode, and might be a
> KVM host, we have to use the physical timers, and require CNTVOFF to be
> zero so as to have a consistent view of time between the physical timers
> and virtual counters.
>
> Any code which may want to alter CNTVOFF (e.g. KVM) will need to ensure
> that it is zeroed when entering the host.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* OMAP4 PM bootloader dependency problems
From: Paul Walmsley @ 2013-01-31 15:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <510A54F8.4090609@ti.com>
Hi,
On Thu, 31 Jan 2013, Rajendra Nayak wrote:
> Throw a pr_warn() at boot only when CONFIG_CPU_IDLE is enabled. Note
> that it isn't enabled by default in omap2plus_defconfig. Also
> throw one when a suspend fails, saying bootloader *could be* a possible
> cause specifying the right version to be used.
In my view, these steps aren't sufficient, for the reasons described in
http://marc.info/?l=linux-omap&m=135964568904053&w=2
Even with CONFIG_CPU_IDLE=n, it's still possible to attempt to enter
full-chip retention idle on OMAP4 via 'echo mem > /sys/power/state', so it
doesn't seem right to restrict a solution to that case.
regards,
- Paul
^ permalink raw reply
* [PATCH v3 01/15] ARM: multi-cluster PM: secondary kernel entry code
From: Santosh Shilimkar @ 2013-01-31 15:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1359445870-18925-2-git-send-email-nicolas.pitre@linaro.org>
On Tuesday 29 January 2013 01:20 PM, Nicolas Pitre wrote:
> CPUs in cluster based systems, such as big.LITTLE, have special needs
> when entering the kernel due to a hotplug event, or when resuming from
> a deep sleep mode.
>
> This is vectorized so multiple CPUs can enter the kernel in parallel
> without serialization.
>
> The mcpm prefix stands for "multi cluster power management", however
> this is usable on single cluster systems as well. Only the basic
> structure is introduced here. This will be extended with later patches.
>
> In order not to complexify things more than they currently have to,
> the planned work to make runtime adjusted MPIDR based indexing and
> dynamic memory allocation for cluster states is postponed to a later
> cycle. The MAX_NR_CLUSTERS and MAX_CPUS_PER_CLUSTER static definitions
> should be sufficient for those systems expected to be available in the
> near future.
>
'mcpm' sounds definitely better than 'bL' ;)
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/arm/Kconfig | 8 ++++
> arch/arm/common/Makefile | 1 +
> arch/arm/common/mcpm_entry.c | 29 +++++++++++++
> arch/arm/common/mcpm_head.S | 86 +++++++++++++++++++++++++++++++++++++++
> arch/arm/include/asm/mcpm_entry.h | 35 ++++++++++++++++
> 5 files changed, 159 insertions(+)
> create mode 100644 arch/arm/common/mcpm_entry.c
> create mode 100644 arch/arm/common/mcpm_head.S
> create mode 100644 arch/arm/include/asm/mcpm_entry.h
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 67874b82a4..200f559c1c 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1584,6 +1584,14 @@ config HAVE_ARM_TWD
> help
> This options enables support for the ARM timer and watchdog unit
>
> +config CLUSTER_PM
> + bool "Cluster Power Management Infrastructure"
> + depends on CPU_V7 && SMP
> + help
> + This option provides the common power management infrastructure
> + for (multi-)cluster based systems, such as big.LITTLE based
> + systems.
> +
> choice
> prompt "Memory split"
> default VMSPLIT_3G
> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
> index e8a4e58f1b..23e85b1fae 100644
> --- a/arch/arm/common/Makefile
> +++ b/arch/arm/common/Makefile
> @@ -13,3 +13,4 @@ obj-$(CONFIG_SHARP_PARAM) += sharpsl_param.o
> obj-$(CONFIG_SHARP_SCOOP) += scoop.o
> obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o
> obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o
> +obj-$(CONFIG_CLUSTER_PM) += mcpm_head.o mcpm_entry.o
> diff --git a/arch/arm/common/mcpm_entry.c b/arch/arm/common/mcpm_entry.c
> new file mode 100644
> index 0000000000..3a6d7e70fd
> --- /dev/null
> +++ b/arch/arm/common/mcpm_entry.c
> @@ -0,0 +1,29 @@
> +/*
> + * arch/arm/common/mcpm_entry.c -- entry point for multi-cluster PM
> + *
> + * Created by: Nicolas Pitre, March 2012
> + * Copyright: (C) 2012-2013 Linaro Limited
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +
> +#include <asm/mcpm_entry.h>
> +#include <asm/barrier.h>
> +#include <asm/proc-fns.h>
> +#include <asm/cacheflush.h>
> +
> +extern volatile unsigned long mcpm_entry_vectors[MAX_NR_CLUSTERS][MAX_CPUS_PER_CLUSTER];
> +
> +void mcpm_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr)
> +{
> + unsigned long val = ptr ? virt_to_phys(ptr) : 0;
May be extra line would be good here.
Patch looks fine to my eyes otherwise.
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
^ permalink raw reply
* [PATCH v2 19/27] pci: PCIe driver for Marvell Armada 370/XP systems
From: Thomas Petazzoni @ 2013-01-31 15:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130131153625.GB23505@n2100.arm.linux.org.uk>
Dear Russell King - ARM Linux,
On Thu, 31 Jan 2013 15:36:25 +0000, Russell King - ARM Linux wrote:
> > The fixup is already specific to those bridges, since I am just setting
> > pci_sys_data->win_align_io to 64K for the particular buses that are
> > downstream the problematic bridges. So it doesn't affect any other bus
> > on the system, and therefore I don't think this fixup needs to be made
> > specific to a given vendor:device, no?
>
> The pci_sys_data is not specific to one bus. It's specific from the
> root bus downwards, and is shared by all child busses.
Ah, ok, that's the part I was missing.
> The problem is if you have some card or a conventional P2P bridge which
> has 4K windows. If you merely set the alignment to 64K for all bridges,
> then all bridges get this treatment whether or not they need it. That's
> what I'm trying to avoid.
>
> Take, for instance, a cardbus bridge (remember, there are PCI cards which
> can be plugged in to give you a cardbus slot.) I have a device here which
> can be plugged into a cardbus slot which has not just one P2P bridge but
> two, and a bunch of downsteam devices, including VGA, ethernet, USB, PS/2
> etc. (Okay, Linux doesn't support this hardware because of crappy X86
> stuff, despite the fact Windows cope with it just fine.)
>
> There have been cards in the past which have had P2P bridges on them as
> well.
>
> So, simply believing that the only P2P bridges in the system will be
> those on the physical board is a mistake.
Yes, indeed, I understand this. I just thought this pci_sys_data
structure was per-bus. Of course, if it's shared by all buses on the
system, we need a way to apply this fixup only to the Marvell bridges.
Should I just hard-code this special fixup in
pcibios_window_alignment() with a check on VID/PID ?
Thanks,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH v2 19/27] pci: PCIe driver for Marvell Armada 370/XP systems
From: Russell King - ARM Linux @ 2013-01-31 15:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130131164735.3e1d4a88@skate>
On Thu, Jan 31, 2013 at 04:47:35PM +0100, Thomas Petazzoni wrote:
> Dear Russell King - ARM Linux,
>
> On Thu, 31 Jan 2013 15:36:25 +0000, Russell King - ARM Linux wrote:
>
> > > The fixup is already specific to those bridges, since I am just setting
> > > pci_sys_data->win_align_io to 64K for the particular buses that are
> > > downstream the problematic bridges. So it doesn't affect any other bus
> > > on the system, and therefore I don't think this fixup needs to be made
> > > specific to a given vendor:device, no?
> >
> > The pci_sys_data is not specific to one bus. It's specific from the
> > root bus downwards, and is shared by all child busses.
>
> Ah, ok, that's the part I was missing.
>
> > The problem is if you have some card or a conventional P2P bridge which
> > has 4K windows. If you merely set the alignment to 64K for all bridges,
> > then all bridges get this treatment whether or not they need it. That's
> > what I'm trying to avoid.
> >
> > Take, for instance, a cardbus bridge (remember, there are PCI cards which
> > can be plugged in to give you a cardbus slot.) I have a device here which
> > can be plugged into a cardbus slot which has not just one P2P bridge but
> > two, and a bunch of downsteam devices, including VGA, ethernet, USB, PS/2
> > etc. (Okay, Linux doesn't support this hardware because of crappy X86
> > stuff, despite the fact Windows cope with it just fine.)
> >
> > There have been cards in the past which have had P2P bridges on them as
> > well.
> >
> > So, simply believing that the only P2P bridges in the system will be
> > those on the physical board is a mistake.
>
> Yes, indeed, I understand this. I just thought this pci_sys_data
> structure was per-bus. Of course, if it's shared by all buses on the
> system, we need a way to apply this fixup only to the Marvell bridges.
>
> Should I just hard-code this special fixup in
> pcibios_window_alignment() with a check on VID/PID ?
Yes please.
^ permalink raw reply
* [PATCH] ARM: let CPUs not being able to run in ARM mode enter in THUMB mode
From: Uwe Kleine-König @ 2013-01-31 15:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130129124427.GB1974@linaro.org>
Hello Dave,
On Tue, Jan 29, 2013 at 12:44:27PM +0000, Dave Martin wrote:
> On Fri, Jan 11, 2013 at 12:39:57PM +0100, Uwe Kleine-K?nig wrote:
> > Some ARM cores are not capable to run in ARM mode (e.g. Cortex-M3). So
> > obviously these cannot enter the kernel in ARM mode. Make an exception
> > for them and let them enter in THUMB mode.
> >
> > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > ---
> > arch/arm/kernel/head-nommu.S | 8 +++++++-
> > arch/arm/kernel/head.S | 8 +++++++-
> > arch/arm/mm/Kconfig | 6 ++++++
> > 3 files changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
> > index 3782320..ae7ed46 100644
> > --- a/arch/arm/kernel/head-nommu.S
> > +++ b/arch/arm/kernel/head-nommu.S
> > @@ -32,15 +32,21 @@
> > * numbers for r1.
> > *
> > */
> > - .arm
> >
> > __HEAD
> > +
> > +#ifdef CONFIG_THUMBONLY_CPU
> > + .thumb
> > +ENTRY(stext)
> > +#else
> > + .arm
> > ENTRY(stext)
> >
> > THUMB( adr r9, BSYM(1f) ) @ Kernel is always entered in ARM.
> > THUMB( bx r9 ) @ If this is a Thumb-2 kernel,
> > THUMB( .thumb ) @ switch to Thumb now.
> > THUMB(1: )
> > +#endif
>
> The behaviour is that we start the file is kernel entry state, then
> we switch to kernel run state.
>
> The switch is only needed for kernels which run on CPUs supporting
> both states, where the run state is not ARM.
>
> So, it would be neater for these entry sequences to look something
> like:
>
> HAS_ARM(.arm) @ because -mthumb is default for Thumb kernels anyway
>
> ENTRY(stext)
> HAS_ARM(THUMB( @ code to switch to Thumb ))
>
> @ real code starts here, in run state.
>
>
>
> Where
>
> #ifdef CONFIG_THUMBONLY_CPU
> #define HAS_ARM(x...)
> #else
> #define HAS_ARM(x...) x
> #endif
>
> (I haven't read the whole thread yet, so decisions about what
> config variables and macro names should be used may be overridden
> by other people...)
I don't agree on better readability, the result would look like:
__HEAD
HAS_ARM(.arm)
ENTRY(stext)
HAS_ARM(THUMB(adr r9, BSYM(1f) )
HAS_ARM(THUMB(bx r9 )
HAS_ARM(THUMB(.thumb )
HAS_ARM(THUMB(1: )
in contrast to:
__HEAD
#ifdef CONFIG_CPU_THUMBONLY
.thumb
ENTRY(stext)
#else
.arm
ENTRY(stext)
THUMB( adr r9, BSYM(1f) ) @ Kernel is always entered in ARM.
THUMB( bx r9 ) @ If this is a Thumb-2 kernel,
THUMB( .thumb ) @ switch to Thumb now.
THUMB(1: )
#endif
(modulo comments and indention maybe). Even though it uses an #ifdef I
consider the latter variant more readable. Maybe it's only me? Other
than that I'd probably choose a longer name instead of HAS_ARM to make
it more self-documenting what it is about (something with _ISA_ in its
name). This obviously makes readability worse.
So I'm still happy with the version I sent.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH v3 02/15] ARM: mcpm: introduce the CPU/cluster power API
From: Santosh Shilimkar @ 2013-01-31 15:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1359445870-18925-3-git-send-email-nicolas.pitre@linaro.org>
On Tuesday 29 January 2013 01:20 PM, Nicolas Pitre wrote:
> This is the basic API used to handle the powering up/down of individual
> CPUs in a (multi-)cluster system. The platform specific backend
> implementation has the responsibility to also handle the cluster level
> power as well when the first/last CPU in a cluster is brought up/down.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
^ permalink raw reply
* [PATCH v2 2/2] ARM: memory: define TASK_UNMAPPED_BASE in terms of TASK_SIZE
From: Nicolas Pitre @ 2013-01-31 15:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1359644949-18550-2-git-send-email-will.deacon@arm.com>
On Thu, 31 Jan 2013, Will Deacon wrote:
> TASK_UNMAPPED_BASE is defined directly in terms of PAGE_OFFSET, which is
> confusing given that the modules area sits between here and TASK_SIZE
> and is not available for user allocations.
>
> This patch defines TASK_UNMAPPED_BASE in terms of TASK_SIZE instead and
> fixes a bug introduced by 394ef6403abc ("mm: use vm_unmapped_area() on
> arm architecture") whereby TASK_UNMAPPED_BASE is no longer page-aligned
> for bottom-up mmap, causing get_unmapped_area to choke on misaligned
> addresses.
>
> Reported-by: Christoffer Dall <cdall@cs.columbia.edu>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
Acked-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/arm/include/asm/memory.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 924320f..64c770d 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -37,7 +37,7 @@
> */
> #define PAGE_OFFSET UL(CONFIG_PAGE_OFFSET)
> #define TASK_SIZE (UL(CONFIG_PAGE_OFFSET) - UL(SZ_16M))
> -#define TASK_UNMAPPED_BASE (UL(CONFIG_PAGE_OFFSET) / 3)
> +#define TASK_UNMAPPED_BASE ALIGN(TASK_SIZE / 3, SZ_16M)
>
> /*
> * The maximum size of a 26-bit user space task.
> --
> 1.8.0
>
^ permalink raw reply
* Failure to boot...
From: Tony Lindgren @ 2013-01-31 16:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <510A82FE.6050803@ti.com>
* Santosh Shilimkar <santosh.shilimkar@ti.com> [130131 06:45]:
> On Thursday 31 January 2013 07:57 PM, Russell King - ARM Linux wrote:
> >>>
> >>># 13 Jan 2013: Update for OMAP moving to multiplatform
> >>>CONFIG_ARCH_MULTIPLATFORM=y
> >>>CONFIG_ARCH_MULTI_V7=y
> >>>
> >>>don't fix it either, it needs more options defined...
Just trying to figure out what was the missing entry in case
others have the same problem.
In addition to the ones above, I had also CONFIG_ARCH_OMAP2PLUS=y
listed in my comments. Did you also need something else added
manually?
Regards,
Tony
^ permalink raw reply
* Failure to boot...
From: Fabio Estevam @ 2013-01-31 16:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130131142758.GS2637@n2100.arm.linux.org.uk>
On Thu, Jan 31, 2013 at 12:27 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> too. It also looks like Versatile Express support _can't_ be disabled in
> a multiplatform kernel, which is ludicrous.
Should we go with this approach?
https://patchwork.kernel.org/patch/1774521/
^ permalink raw reply
* [PATCH v2 19/27] pci: PCIe driver for Marvell Armada 370/XP systems
From: Thomas Petazzoni @ 2013-01-31 16:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAErSpo5NTyrHv8AQkw3hYpJRiZsk1HKHP3jP_MMpEVVhCgjKYg@mail.gmail.com>
Dear Bjorn Helgaas,
On Wed, 30 Jan 2013 11:52:15 -0700, Bjorn Helgaas wrote:
> The secondary/subordinate registers effectively define a bus number
> aperture that tells the bridge which transactions to claim and forward
> downstream. When enumerating devices, we may update the subordinate
> bus number to widen the aperture so we can enumerate an arbitrary tree
> behind the bridge. When we're finished, we'll probably narrow it by
> updating the subordinate again, so the unused bus number space can be
> used for other bridges. I don't know the exact details of the
> algorithm, and they're likely to change anyway, but pci_scan_bridge()
> is where most of it happens.
>
> It looks like your current system doesn't support trees below the
> bridges, but hopefully we can make it so the generic enumeration
> algorithms still work.
The PCI-to-PCI bridge specification says that the Primary Bus Number
Register, Secondary Bus Number Register and Subordinate Bus Number
Register of the PCI configuration space of a PCI-to-PCI bridge should
all be set to 0 after reset.
Until now, I was forcing a specific value of the Secondary Bus Number
and Subordinate Bus Number (1 for my first bridge, 2 for my second
bridge, etc.).
Following you're recommendation, I've changed this, and left those
values initialized to 0 by default, in order to let Linux set correct
values. Yes, Linux does assign appropriate values in the Secondary Bus
Number Register. But before that Linux also complains loudly that the
bridge configuration is invalid:
pci 0000:00:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
pci 0000:00:02.0: bridge configuration invalid ([bus 00-00]), reconfiguring
pci 0000:00:03.0: bridge configuration invalid ([bus 00-00]), reconfiguring
pci 0000:00:04.0: bridge configuration invalid ([bus 00-00]), reconfiguring
pci 0000:00:05.0: bridge configuration invalid ([bus 00-00]), reconfiguring
pci 0000:00:06.0: bridge configuration invalid ([bus 00-00]), reconfiguring
Looking at the code, we have:
/* Check if setup is sensible at all */
if (!pass &&
(primary != bus->number || secondary <= bus->number ||
secondary > subordinate)) {
dev_info(&dev->dev, "bridge configuration invalid ([bus %02x-%02x]), reconfiguring\n",
secondary, subordinate);
broken = 1;
}
Due to the default values of the Primary Bus Number Register, Secondary
Bus Number Register and Subordinate Bus Number Register, we have:
primary = 0
secondary = 0
subordinate = 0
We are enumerating the root bus, so bus->number = 0. Therefore:
* The test primary != bus->number is false, so it's not the problem.
* secondary <= bus->number is true, because secondary = 0, and
bus->number = 0. It is the problem.
* secondary > subordinate is false.
So I'm not sure what to do with this...
Thoas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH] cpufreq: exynos: Broadcast frequency change notifications for all cores
From: Tomasz Figa @ 2013-01-31 16:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKohpomEcrPa7UnZmtTouxN6JXxcaa7dBstMGUWjXGL0n1+=Ng@mail.gmail.com>
On Thursday 31 of January 2013 20:46:10 Viresh Kumar wrote:
> On 31 January 2013 20:34, Tomasz Figa <t.figa@samsung.com> wrote:
> > Well, the fact that it isn't used at the moment doesn't mean that it
> > shouldn't be set correctly. The field is present in the structure and
> > has a set of defined values - one of which should be selected. For
> > example, I can imagine some governor taking this information into
> > account.
> Governors already take this information but from a different variable:
> policy->cpus.
>
> Look at the patch which added it:
>
> commit 3b2d99429e3386b6e2ac949fc72486509c8bbe36
> Author: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> 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
>
> Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
> drivers/acpi/processor_perflib.c | 228
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++ include/acpi/processor.h |
> 27 ++++++++++++-
> include/linux/cpufreq.h | 4 ++
> 3 files changed, 258 insertions(+), 1 deletion(-)
>
>
> It was clearly for ACPI, but was probably named badly and we people
> got confused that it is for our use.
Hmm. Now as I think of it, there might be another confusing aspect:
Could you explain what "sw coordination" and "any coordination" mean?
I mean specifically cpufreq_policy.cpus and cpufreq_policy.related_cpus
masks.
Best regards,
--
Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Linux Platform
^ permalink raw reply
* [PATCH 5/5] ARM: OMAP3: Update clocksource timer selection
From: Jon Hunter @ 2013-01-31 16:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <510A3493.6050508@compulab.co.il>
On 01/31/2013 03:08 AM, Igor Grinberg wrote:
> On 01/30/13 19:04, Jon Hunter wrote:
>> When booting with device-tree for OMAP3 and AM335x devices and a gptimer
>> is used as the clocksource (which is always the case for AM335x), a
>> gptimer located in a power domain that is not always-on is selected.
>> Ideally we should use a gptimer located in a power domain that is always
>> on (such as the wake-up domain) so that time can be maintained during a
>> kernel suspend without keeping on additional power domains unnecessarily.
>>
>> In order to fix this so that we can select a gptimer located in a power
>> domain that is always-on, the following changes were made ...
>> 1. Currently, only when selecting a gptimer to use for a clockevent
>> timer, do we pass a timer property that can be used to select a
>> specific gptimer. Change this so that we can pass a property when
>> selecting a gptimer to use for a clocksource timer too.
>> 2. Currently, when selecting either a gptimer to use for a clockevent
>> timer or a clocksource timer and no timer property is passed, then
>> the first available timer is selected regardless of the properties
>> it has. Change this so that if no properties are passed, then a timer
>> that does not have additional features (such as always-on, dsp-irq,
>> pwm, and secure) is selected.
>>
>> Please note that using a gptimer for both clocksource and clockevents
>> can have a system power impact during idle. The reason being is that
>> OMAP and AMxxx devices typically only have one gptimer in a power domain
>> that is always-on. Therefore when the kernel is idle both the clocksource
>> and clockevent timers will be active and this will keep additional power
>> domains on. During kernel suspend, only the clocksource timer is active
>> and therefore, it is better to use a gptimer in a power domain that is
>> always-on for clocksource.
>
> This should explain the gptimer number switch in the
> #if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX)
> section below, right?
> I would really like to see that clearly stated in the commit message.
> For instance:
> ... it is better to use a gptimer in a power domain that is
> always-on for clocksource. Therefore we switch to use the first gptimer
> for clocksource and the second for clockevents.
Yes exactly. Good point I can make that bit explicit.
>>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>
> Apart from above,
> Acked-by: Igor Grinberg <grinberg@compulab.co.il>
Thanks
Jon
^ permalink raw reply
* [PATCH v3 03/15] ARM: mcpm: introduce helpers for platform coherency exit/setup
From: Santosh Shilimkar @ 2013-01-31 16:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1359445870-18925-4-git-send-email-nicolas.pitre@linaro.org>
On Tuesday 29 January 2013 01:20 PM, Nicolas Pitre wrote:
> From: Dave Martin <dave.martin@linaro.org>
>
> This provides helper methods to coordinate between CPUs coming down
> and CPUs going up, as well as documentation on the used algorithms,
> so that cluster teardown and setup
> operations are not done for a cluster simultaneously.
>
> For use in the power_down() implementation:
> * __mcpm_cpu_going_down(unsigned int cluster, unsigned int cpu)
> * __mcpm_outbound_enter_critical(unsigned int cluster)
> * __mcpm_outbound_leave_critical(unsigned int cluster)
> * __mcpm_cpu_down(unsigned int cluster, unsigned int cpu)
>
> The power_up_setup() helper should do platform-specific setup in
> preparation for turning the CPU on, such as invalidating local caches
> or entering coherency. It must be assembler for now, since it must
> run before the MMU can be switched on. It is passed the affinity level
> which should be initialized.
>
> Because the mcpm_sync_struct content is looked-up and modified
> with the cache enabled or disabled depending on the code path, it is
> crucial to always ensure proper cache maintenance to update main memory
> right away. Therefore, any cached write must be followed by a cache
> clean operation and any cached read must be preceded by a cache
> invalidate operation (actually a cache flush i.e. clean+invalidate to
> avoid discarding possible concurrent writes) on the accessed memory.
>
> Also, in order to prevent a cached writer from interfering with an
> adjacent non-cached writer, we ensure each state variable is located to
> a separate cache line.
>
> Thanks to Nicolas Pitre and Achin Gupta for the help with this
> patch.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> ---
[..]
> diff --git a/arch/arm/common/mcpm_entry.c b/arch/arm/common/mcpm_entry.c
> index c8c0e2113e..2b83121966 100644
> --- a/arch/arm/common/mcpm_entry.c
> +++ b/arch/arm/common/mcpm_entry.c
> @@ -18,6 +18,7 @@
> #include <asm/proc-fns.h>
> #include <asm/cacheflush.h>
> #include <asm/idmap.h>
> +#include <asm/cputype.h>
>
> extern volatile unsigned long mcpm_entry_vectors[MAX_NR_CLUSTERS][MAX_CPUS_PER_CLUSTER];
>
[...]
> +/*
> + * Ensure preceding writes to *p by other CPUs are visible to
> + * subsequent reads by this CPU. We must be careful not to
> + * discard data simultaneously written by another CPU, hence the
> + * usage of flush rather than invalidate operations.
> + */
> +static void __sync_range_r(volatile void *p, size_t size)
> +{
> + char *_p = (char *)p;
> +
> +#ifdef CONFIG_OUTER_CACHE
> + if (outer_cache.flush_range) {
> +
You don't need above #ifdef. In case of non-outer
cache the function pointer is null anyways.
/*
> + * Ensure dirty data migrated from other CPUs into our cache
> + * are cleaned out safely before the outer cache is cleaned:
> + */
> + __cpuc_clean_dcache_area(_p, size);
> +
> + /* Clean and invalidate stale data for *p from outer ... */
> + outer_flush_range(__pa(_p), __pa(_p + size));
> + }
> +#endif
> +
> + /* ... and inner cache: */
> + __cpuc_flush_dcache_area(_p, size);
This will be un-necessary when inner cache is available, no ?
May be you can re-arrange the code like below, unless and until
you would like to invalidate any speculative fetches during the
outer_flush_range()
__cpuc_clean_dcache_area(_p, size);
if (outer_cache.flush_range)
outer_flush_range(__pa(_p), __pa(_p + size));
Rest of the patch looks fine to me.
Regards,
Santosh
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox