linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/14] CPU idle for Armada XP
@ 2014-03-25 22:48 Gregory CLEMENT
  2014-03-25 22:48 ` [PATCH v5 01/14] ARM: PJ4B: Add cpu_suspend/cpu_resume hooks for PJ4B Gregory CLEMENT
                   ` (14 more replies)
  0 siblings, 15 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

This patch set adds the CPU idle support for Armada XP and prepares
the support for Armada 370. This was based on the work of Nadav
Haklai.

As for the last version there were many changes done, see the
changelog for the details. And again I think that I should have
managed to comply with all the reviewer wishes.

A notable change is that patches 7 and 8 are here to have a coherent
series, but they will be part of a different series which will be send
soon, so don't focus too much on it. These 2 patches introduce a new
compatible string for PMSU which will be needed for CPU idle
(obviously) but also for the SMP support for Armada 375 and Armada
38x. So instead of having SMP for Armada 375/38x depending on CPU idle
for Armada XP or the opposite we choose to submit soon a small series
needed for those two bigger series.

Most of the patches modify the mvebu code in order to prepare the
support for CPU idle, hence the patches 2 to 6 and from 9 to 12 should
go to mvebu subsystem (and then arm-soc).

The first patch should go through ARM subsystem and should be taken by
Russell King. I made few change on it following Lorenzo advice and
now it will reuse the cpu v7 suspend and resume function and just do
specific operation before calling them.

The 13th patch 'cpuidle: mvebu: Add initial cpu idle support for
Armada 370/XP SoC' is the only one who should go to the cpuidle
subsystem. But of course I would like that Daniel Lezcano or Rafael
J. Wysocki have a look on the whole series and especially patches 10,
12 and 14.

The 14th patch should go to mvebu subsystem.

The whole series is also available in the branch CPU-idle-ArmadaXP-v5
at https://github.com/MISL-EBU-System-SW/mainline-public.git

Thanks,

Changelog:
v4 -> v5:

* Call v7 version of suspend and resume from the pj4b callback instead
  of copying the code, following Lorenzo advice.

* Split the low level functions to manipulate HW coherency

* Reorder the included headers in cpuidle-armada-370-xp.c

* Added comment in the inline asm part of armada_370_xp_cpu_suspend
  function where the CR_C is restored.

* Added comment about the fact that Armada XP can us ldrex/strex
  without MMU enabled

* Fixed commit logs

* Added the acked-by from Thomas Petazzoni

* Extend the pmsu registers instead of adding a new set of register

* Move the pm initialization outside of the board file in a
  arch_initcall

* Move most of the architecture specific code in arch/arm/mach-mvebu
  and use platform device data file to pass the callback as suggested
  by Daniel.

* Added the tlb flush if the wfi failed.

* Check the cpu_suspend return value

* rebased on mvebu/for-next to be in sync all the change made in mach-mvebu

v3 -> v4:

* factorized the code in coherency_ll.S and make it autodetect as mush
  as possible

* reordered the introduction of the device tree binding

* removed all the EXPORT_SYMBOL_GPL as the driver can only be built
  into the kernel and never be built as a module.

* moved the armada_370_xp_pmsu_enable_l2_powerdown_onidle function in
  armada_370_cp.c file during the initialization of the platform.

* fixed various coding style issue and typos pointed by Thomas

* fixed all the coding issue style, made the comments more coherent
  and add more comment in the suspend-armada-370-xp.S file.

* moved all the device tree related check from
  armada_370_xp_cpuidle_probe to armada_370_xp_dt_init.

* used cpu_pm_enter() instead of directly calling platform code in
  Armada_370_xp_enter_idle.

* convert the sequence to disable the coherency to the one used in
  TC2.

* Rebased on v3.14-rc1

v2 -> v3:

* Converted the driver to use module_platform_driver. This lead to the
  introduction of a new patch (PATCH 11). Pointed by Daniel Lezcano.

* Used PUIDLE_DRIVER_FLAGS_MASK to store the deep idle information,
  suggested by Daniel Lezcano.

* Removed cpu_init call from armada_370_xp_enter_idle
  function. Pointed by Lorenzo Pieralisi.

* Rebased on v3.12-rc5


v1 -> v2:

* Removed the pm_level kernel parameter. As Kevin Hilman pointed, its
  usage can be replaced by using
  /sys/devices/system/cpu/cpu*/cpuidle/state*/disable or the kernel
  parameter cpuidle.off.

* Used BIT() macro (reported by Ezequiel)

* Made the function more readable the
  armada_370_xp_pmsu_idle_prepare() function (reported by Thomas)

* Moved the config entry in Kconfig.arm, and rename the config symbol
  according the pattern used by other arm cpu: ARM_"soc name"_CPUIDLE

* Moved the build rule under the new ARM SoC section in the Makefile

* Rebased on Linus Torvalds master branch of Thursday September 12

Gregory CLEMENT (14):
  ARM: PJ4B: Add cpu_suspend/cpu_resume hooks for PJ4B
  ARM: mvebu: remove the address parameter for ll_set_cpu_coherent
  ARM: mvebu: ll_set_cpu_coherent always uses the current CPU
  ARM: mvebu: Remove the unused argument of set_cpu_coherent()
  ARM: mvebu: Split low level functions to manipulate HW coherency
  ARM: mvebu: Low level function to disable HW coherency support
  ARM: mvebu: Extend the pmsu registers
  ARM: dts: mvebu: Introduce a new compatible for the PMSU node
  ARM: mvebu: Allow to power down L2 cache controller in idle mode
  ARM: mvebu: Add the PMSU related part of the cpu idle functions
  ARM: mvebu: Set the start address of a CPU in a separate function
  ARM: mvebu: Register notifier callback for the cpuidle transition
  cpuidle: mvebu: Add initial CPU idle support for Armada 370/XP SoC
  ARM: mvebu: register the cpuidle driver for the Armada XP SoCs

 .../devicetree/bindings/arm/armada-370-xp-pmsu.txt |  14 +-
 arch/arm/boot/dts/armada-xp.dtsi                   |  11 +-
 arch/arm/mach-mvebu/coherency.c                    |  16 +-
 arch/arm/mach-mvebu/coherency.h                    |   2 +-
 arch/arm/mach-mvebu/coherency_ll.S                 | 124 +++++++---
 arch/arm/mach-mvebu/headsmp.S                      |  15 +-
 arch/arm/mach-mvebu/platsmp.c                      |   2 +-
 arch/arm/mach-mvebu/pmsu.c                         | 251 ++++++++++++++++++++-
 arch/arm/mm/proc-v7.S                              |  28 ++-
 drivers/cpuidle/Kconfig.arm                        |   5 +
 drivers/cpuidle/Makefile                           |   1 +
 drivers/cpuidle/cpuidle-armada-370-xp.c            |  93 ++++++++
 12 files changed, 490 insertions(+), 72 deletions(-)
 create mode 100644 drivers/cpuidle/cpuidle-armada-370-xp.c

-- 
1.8.1.2

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v5 01/14] ARM: PJ4B: Add cpu_suspend/cpu_resume hooks for PJ4B
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
@ 2014-03-25 22:48 ` Gregory CLEMENT
  2014-03-25 22:48 ` [PATCH v5 02/14] ARM: mvebu: remove the address parameter for ll_set_cpu_coherent Gregory CLEMENT
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

PJ4B needs extra instructions for suspend and resume, so instead of
using the armv7 version, this commit introduces specific versions for
PJ4B.

Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/mm/proc-v7.S | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index bd1781979a39..3c49ac86d9d2 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -169,9 +169,31 @@ ENDPROC(cpu_pj4b_do_idle)
 	globl_equ	cpu_pj4b_do_idle,  	cpu_v7_do_idle
 #endif
 	globl_equ	cpu_pj4b_dcache_clean_area,	cpu_v7_dcache_clean_area
-	globl_equ	cpu_pj4b_do_suspend,	cpu_v7_do_suspend
-	globl_equ	cpu_pj4b_do_resume,	cpu_v7_do_resume
-	globl_equ	cpu_pj4b_suspend_size,	cpu_v7_suspend_size
+#ifdef CONFIG_ARM_CPU_SUSPEND
+ENTRY(cpu_pj4b_do_suspend)
+	stmfd	sp!, {r6 - r10}
+	mrc	p15, 1, r6, c15, c1, 0  @ save CP15 - extra features
+	mrc	p15, 1, r7, c15, c2, 0	@ save CP15 - Aux Func Modes Ctrl 0
+	mrc	p15, 1, r8, c15, c1, 2	@ save CP15 - Aux Debug Modes Ctrl 2
+	mrc	p15, 1, r9, c15, c1, 1  @ save CP15 - Aux Debug Modes Ctrl 1
+	mrc	p15, 0, r10, c9, c14, 0  @ save CP15 - PMC
+	stmia	r0!, {r6 - r10}
+	ldmfd	sp!, {r6 - r10}
+	b cpu_v7_do_suspend
+ENDPROC(cpu_pj4b_do_suspend)
+
+ENTRY(cpu_pj4b_do_resume)
+	ldmia	r0!, {r6 - r10}
+	mcr	p15, 1, r6, c15, c1, 0  @ save CP15 - extra features
+	mcr	p15, 1, r7, c15, c2, 0	@ save CP15 - Aux Func Modes Ctrl 0
+	mcr	p15, 1, r8, c15, c1, 2	@ save CP15 - Aux Debug Modes Ctrl 2
+	mcr	p15, 1, r9, c15, c1, 1  @ save CP15 - Aux Debug Modes Ctrl 1
+	mcr	p15, 0, r10, c9, c14, 0  @ save CP15 - PMC
+	b cpu_v7_do_resume
+ENDPROC(cpu_pj4b_do_resume)
+#endif
+.globl	cpu_pj4b_suspend_size
+.equ	cpu_pj4b_suspend_size, 4 * 14
 
 #endif
 
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v5 02/14] ARM: mvebu: remove the address parameter for ll_set_cpu_coherent
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
  2014-03-25 22:48 ` [PATCH v5 01/14] ARM: PJ4B: Add cpu_suspend/cpu_resume hooks for PJ4B Gregory CLEMENT
@ 2014-03-25 22:48 ` Gregory CLEMENT
  2014-03-25 22:48 ` [PATCH v5 03/14] ARM: mvebu: ll_set_cpu_coherent always uses the current CPU Gregory CLEMENT
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

In order to be able to deal with the MMU enabled and the MMU disabled
cases, the base address of the coherency registers was passed to the
function. The address by itself was not interesting as it can't change
for a given SoC, the only thing we need is to have a distinction
between the physical or the virtual address.

This patch add a check of the MMU bit to choose the accurate address,
then the calling function doesn't have to pass this information.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/mach-mvebu/coherency.c    |  6 +++---
 arch/arm/mach-mvebu/coherency_ll.S | 22 ++++++++++++++++++++--
 arch/arm/mach-mvebu/headsmp.S      |  9 ---------
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-mvebu/coherency.c b/arch/arm/mach-mvebu/coherency.c
index 4e9d58148ca7..88dd507221fc 100644
--- a/arch/arm/mach-mvebu/coherency.c
+++ b/arch/arm/mach-mvebu/coherency.c
@@ -30,7 +30,7 @@
 #include "coherency.h"
 
 unsigned long coherency_phys_base;
-static void __iomem *coherency_base;
+void __iomem *coherency_base;
 static void __iomem *coherency_cpu_base;
 
 /* Coherency fabric registers */
@@ -44,7 +44,7 @@ static struct of_device_id of_coherency_table[] = {
 };
 
 /* Function defined in coherency_ll.S */
-int ll_set_cpu_coherent(void __iomem *base_addr, unsigned int hw_cpu_id);
+int ll_set_cpu_coherent(unsigned int hw_cpu_id);
 
 int set_cpu_coherent(unsigned int hw_cpu_id, int smp_group_id)
 {
@@ -54,7 +54,7 @@ int set_cpu_coherent(unsigned int hw_cpu_id, int smp_group_id)
 		return 1;
 	}
 
-	return ll_set_cpu_coherent(coherency_base, hw_cpu_id);
+	return ll_set_cpu_coherent(hw_cpu_id);
 }
 
 static inline void mvebu_hwcc_sync_io_barrier(void)
diff --git a/arch/arm/mach-mvebu/coherency_ll.S b/arch/arm/mach-mvebu/coherency_ll.S
index ee7598fe75db..1f2bcd4b5424 100644
--- a/arch/arm/mach-mvebu/coherency_ll.S
+++ b/arch/arm/mach-mvebu/coherency_ll.S
@@ -21,13 +21,27 @@
 #define ARMADA_XP_CFB_CFG_REG_OFFSET 0x4
 
 #include <asm/assembler.h>
+#include <asm/cp15.h>
 
 	.text
 /*
- * r0: Coherency fabric base register address
- * r1: HW CPU id
+ * r0: HW CPU id
  */
 ENTRY(ll_set_cpu_coherent)
+	mrc	p15, 0, r1, c1, c0, 0
+	tst	r1, #CR_M @ Check MMU bit enabled
+	bne	1f
+
+	/* use physical address of the coherency register*/
+	adr	r0, 3f
+	ldr	r3, [r0]
+	ldr	r0, [r0, r3]
+	b	2f
+1:
+	/* use virtual address of the coherency register*/
+	ldr	r0, =coherency_base
+	ldr	r0, [r0]
+2:
 	/* Create bit by cpu index */
 	mov	r3, #(1 << 24)
 	lsl	r1, r3, r1
@@ -56,3 +70,7 @@ ARM_BE8(rev	r1, r1)
 	mov	r0, #0
 	mov	pc, lr
 ENDPROC(ll_set_cpu_coherent)
+
+	.align 2
+3:
+	.long	coherency_phys_base - .
diff --git a/arch/arm/mach-mvebu/headsmp.S b/arch/arm/mach-mvebu/headsmp.S
index 3dd80df428f7..f30bc8d78871 100644
--- a/arch/arm/mach-mvebu/headsmp.S
+++ b/arch/arm/mach-mvebu/headsmp.S
@@ -31,11 +31,6 @@
 ENTRY(armada_xp_secondary_startup)
  ARM_BE8(setend	be )			@ go BE8 if entered LE
 
-	/* Get coherency fabric base physical address */
-	adr	r0, 1f
-	ldr	r1, [r0]
-	ldr	r0, [r0, r1]
-
 	/* Read CPU id */
 	mrc     p15, 0, r1, c0, c0, 5
 	and     r1, r1, #0xF
@@ -45,7 +40,3 @@ ENTRY(armada_xp_secondary_startup)
 	b	secondary_startup
 
 ENDPROC(armada_xp_secondary_startup)
-
-	.align 2
-1:
-	.long	coherency_phys_base - .
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v5 03/14] ARM: mvebu: ll_set_cpu_coherent always uses the current CPU
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
  2014-03-25 22:48 ` [PATCH v5 01/14] ARM: PJ4B: Add cpu_suspend/cpu_resume hooks for PJ4B Gregory CLEMENT
  2014-03-25 22:48 ` [PATCH v5 02/14] ARM: mvebu: remove the address parameter for ll_set_cpu_coherent Gregory CLEMENT
@ 2014-03-25 22:48 ` Gregory CLEMENT
  2014-03-26 11:52   ` Sebastian Hesselbarth
  2014-03-25 22:48 ` [PATCH v5 04/14] ARM: mvebu: Remove the unused argument of set_cpu_coherent() Gregory CLEMENT
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

ll_set_cpu_coherent is always used on the current CPU, so instead of
passing the CPU id as argument, ll_set_cpu_coherent() can find it by
itself.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/mach-mvebu/coherency.c    | 10 +++++-----
 arch/arm/mach-mvebu/coherency.h    |  2 +-
 arch/arm/mach-mvebu/coherency_ll.S | 12 ++++++------
 arch/arm/mach-mvebu/headsmp.S      |  4 ----
 arch/arm/mach-mvebu/platsmp.c      |  2 +-
 5 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-mvebu/coherency.c b/arch/arm/mach-mvebu/coherency.c
index 88dd507221fc..51010dbbf7e4 100644
--- a/arch/arm/mach-mvebu/coherency.c
+++ b/arch/arm/mach-mvebu/coherency.c
@@ -44,17 +44,17 @@ static struct of_device_id of_coherency_table[] = {
 };
 
 /* Function defined in coherency_ll.S */
-int ll_set_cpu_coherent(unsigned int hw_cpu_id);
+int ll_set_cpu_coherent(void);
 
-int set_cpu_coherent(unsigned int hw_cpu_id, int smp_group_id)
+int set_cpu_coherent(int smp_group_id)
 {
 	if (!coherency_base) {
-		pr_warn("Can't make CPU %d cache coherent.\n", hw_cpu_id);
+		pr_warn("Can't make current CPU cache coherent.\n");
 		pr_warn("Coherency fabric is not initialized\n");
 		return 1;
 	}
 
-	return ll_set_cpu_coherent(hw_cpu_id);
+	return ll_set_cpu_coherent();
 }
 
 static inline void mvebu_hwcc_sync_io_barrier(void)
@@ -140,7 +140,7 @@ int __init coherency_init(void)
 		sync_cache_w(&coherency_phys_base);
 		coherency_base = of_iomap(np, 0);
 		coherency_cpu_base = of_iomap(np, 1);
-		set_cpu_coherent(cpu_logical_map(smp_processor_id()), 0);
+		set_cpu_coherent(0);
 		of_node_put(np);
 	}
 
diff --git a/arch/arm/mach-mvebu/coherency.h b/arch/arm/mach-mvebu/coherency.h
index 760226c41353..c7e5df368d98 100644
--- a/arch/arm/mach-mvebu/coherency.h
+++ b/arch/arm/mach-mvebu/coherency.h
@@ -16,7 +16,7 @@
 
 extern unsigned long coherency_phys_base;
 
-int set_cpu_coherent(unsigned int cpu_id, int smp_group_id);
+int set_cpu_coherent(int smp_group_id);
 int coherency_init(void);
 
 #endif	/* __MACH_370_XP_COHERENCY_H */
diff --git a/arch/arm/mach-mvebu/coherency_ll.S b/arch/arm/mach-mvebu/coherency_ll.S
index 1f2bcd4b5424..6cb26b919787 100644
--- a/arch/arm/mach-mvebu/coherency_ll.S
+++ b/arch/arm/mach-mvebu/coherency_ll.S
@@ -24,9 +24,7 @@
 #include <asm/cp15.h>
 
 	.text
-/*
- * r0: HW CPU id
- */
+
 ENTRY(ll_set_cpu_coherent)
 	mrc	p15, 0, r1, c1, c0, 0
 	tst	r1, #CR_M @ Check MMU bit enabled
@@ -43,9 +41,11 @@ ENTRY(ll_set_cpu_coherent)
 	ldr	r0, [r0]
 2:
 	/* Create bit by cpu index */
-	mov	r3, #(1 << 24)
-	lsl	r1, r3, r1
-ARM_BE8(rev	r1, r1)
+	mrc	15, 0, r1, cr0, cr0, 5
+	and	r1, r1, #15
+	mov	r2, #(1 << 24)
+	lsl	r1, r2, r1
+	ARM_BE8(rev	r1, r1)
 
 	/* Add CPU to SMP group - Atomic */
 	add	r3, r0, #ARMADA_XP_CFB_CTL_REG_OFFSET
diff --git a/arch/arm/mach-mvebu/headsmp.S b/arch/arm/mach-mvebu/headsmp.S
index f30bc8d78871..cf7abe6554f7 100644
--- a/arch/arm/mach-mvebu/headsmp.S
+++ b/arch/arm/mach-mvebu/headsmp.S
@@ -31,10 +31,6 @@
 ENTRY(armada_xp_secondary_startup)
  ARM_BE8(setend	be )			@ go BE8 if entered LE
 
-	/* Read CPU id */
-	mrc     p15, 0, r1, c0, c0, 5
-	and     r1, r1, #0xF
-
 	/* Add CPU to coherency fabric */
 	bl	ll_set_cpu_coherent
 	b	secondary_startup
diff --git a/arch/arm/mach-mvebu/platsmp.c b/arch/arm/mach-mvebu/platsmp.c
index a6da03f5b24e..a99d71a747f0 100644
--- a/arch/arm/mach-mvebu/platsmp.c
+++ b/arch/arm/mach-mvebu/platsmp.c
@@ -102,7 +102,7 @@ static void __init armada_xp_smp_prepare_cpus(unsigned int max_cpus)
 
 	set_secondary_cpus_clock();
 	flush_cache_all();
-	set_cpu_coherent(cpu_logical_map(smp_processor_id()), 0);
+	set_cpu_coherent(0);
 
 	/*
 	 * In order to boot the secondary CPUs we need to ensure
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v5 04/14] ARM: mvebu: Remove the unused argument of set_cpu_coherent()
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
                   ` (2 preceding siblings ...)
  2014-03-25 22:48 ` [PATCH v5 03/14] ARM: mvebu: ll_set_cpu_coherent always uses the current CPU Gregory CLEMENT
@ 2014-03-25 22:48 ` Gregory CLEMENT
  2014-03-25 22:48 ` [PATCH v5 05/14] ARM: mvebu: Split low level functions to manipulate HW coherency Gregory CLEMENT
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

set_cpu_coherent() took the SMP group ID as parameter. But this
parameter was never used, and the CPU always uses the SMP group 0. So
we can remove this parameter.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/mach-mvebu/coherency.c | 4 ++--
 arch/arm/mach-mvebu/coherency.h | 2 +-
 arch/arm/mach-mvebu/platsmp.c   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-mvebu/coherency.c b/arch/arm/mach-mvebu/coherency.c
index 51010dbbf7e4..ad61251f7faf 100644
--- a/arch/arm/mach-mvebu/coherency.c
+++ b/arch/arm/mach-mvebu/coherency.c
@@ -46,7 +46,7 @@ static struct of_device_id of_coherency_table[] = {
 /* Function defined in coherency_ll.S */
 int ll_set_cpu_coherent(void);
 
-int set_cpu_coherent(int smp_group_id)
+int set_cpu_coherent(void)
 {
 	if (!coherency_base) {
 		pr_warn("Can't make current CPU cache coherent.\n");
@@ -140,7 +140,7 @@ int __init coherency_init(void)
 		sync_cache_w(&coherency_phys_base);
 		coherency_base = of_iomap(np, 0);
 		coherency_cpu_base = of_iomap(np, 1);
-		set_cpu_coherent(0);
+		set_cpu_coherent();
 		of_node_put(np);
 	}
 
diff --git a/arch/arm/mach-mvebu/coherency.h b/arch/arm/mach-mvebu/coherency.h
index c7e5df368d98..dff16612dd93 100644
--- a/arch/arm/mach-mvebu/coherency.h
+++ b/arch/arm/mach-mvebu/coherency.h
@@ -15,8 +15,8 @@
 #define __MACH_370_XP_COHERENCY_H
 
 extern unsigned long coherency_phys_base;
+int set_cpu_coherent(void);
 
-int set_cpu_coherent(int smp_group_id);
 int coherency_init(void);
 
 #endif	/* __MACH_370_XP_COHERENCY_H */
diff --git a/arch/arm/mach-mvebu/platsmp.c b/arch/arm/mach-mvebu/platsmp.c
index a99d71a747f0..f2f1830063c8 100644
--- a/arch/arm/mach-mvebu/platsmp.c
+++ b/arch/arm/mach-mvebu/platsmp.c
@@ -102,7 +102,7 @@ static void __init armada_xp_smp_prepare_cpus(unsigned int max_cpus)
 
 	set_secondary_cpus_clock();
 	flush_cache_all();
-	set_cpu_coherent(0);
+	set_cpu_coherent();
 
 	/*
 	 * In order to boot the secondary CPUs we need to ensure
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v5 05/14] ARM: mvebu: Split low level functions to manipulate HW coherency
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
                   ` (3 preceding siblings ...)
  2014-03-25 22:48 ` [PATCH v5 04/14] ARM: mvebu: Remove the unused argument of set_cpu_coherent() Gregory CLEMENT
@ 2014-03-25 22:48 ` Gregory CLEMENT
  2014-03-25 22:48 ` [PATCH v5 06/14] ARM: mvebu: Low level function to disable HW coherency support Gregory CLEMENT
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

Actually enabling coherency and adding a CPU on a SMP group are two
different operations which can be done separately. This patch splits
this in two functions.

Moreover as they use common pattern, this patch also creates local low
level functions (ll_get_coherency_base and ll_get_cpuid) to be used by
the exposed functions (ll_add_cpu_to_smp_group and
ll_enable_coherency)

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/mach-mvebu/coherency.c    |  8 ++--
 arch/arm/mach-mvebu/coherency_ll.S | 94 +++++++++++++++++++++++++-------------
 arch/arm/mach-mvebu/headsmp.S      |  6 ++-
 3 files changed, 72 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-mvebu/coherency.c b/arch/arm/mach-mvebu/coherency.c
index ad61251f7faf..6718193ebd3f 100644
--- a/arch/arm/mach-mvebu/coherency.c
+++ b/arch/arm/mach-mvebu/coherency.c
@@ -43,8 +43,9 @@ static struct of_device_id of_coherency_table[] = {
 	{ /* end of list */ },
 };
 
-/* Function defined in coherency_ll.S */
-int ll_set_cpu_coherent(void);
+/* Functions defined in coherency_ll.S */
+int ll_enable_coherency(void);
+void ll_add_cpu_to_smp_group(void);
 
 int set_cpu_coherent(void)
 {
@@ -54,7 +55,8 @@ int set_cpu_coherent(void)
 		return 1;
 	}
 
-	return ll_set_cpu_coherent();
+	ll_add_cpu_to_smp_group();
+	return ll_enable_coherency();
 }
 
 static inline void mvebu_hwcc_sync_io_barrier(void)
diff --git a/arch/arm/mach-mvebu/coherency_ll.S b/arch/arm/mach-mvebu/coherency_ll.S
index 6cb26b919787..c7179a03891d 100644
--- a/arch/arm/mach-mvebu/coherency_ll.S
+++ b/arch/arm/mach-mvebu/coherency_ll.S
@@ -24,52 +24,84 @@
 #include <asm/cp15.h>
 
 	.text
-
-ENTRY(ll_set_cpu_coherent)
+/* Returns with the coherency address in r1 (r0 is untouched)*/
+ENTRY(ll_get_coherency_base)
 	mrc	p15, 0, r1, c1, c0, 0
 	tst	r1, #CR_M @ Check MMU bit enabled
 	bne	1f
 
-	/* use physical address of the coherency register*/
-	adr	r0, 3f
-	ldr	r3, [r0]
-	ldr	r0, [r0, r3]
+	/* use physical address of the coherency register */
+	adr	r1, 3f
+	ldr	r3, [r1]
+	ldr	r1, [r1, r3]
 	b	2f
 1:
-	/* use virtual address of the coherency register*/
-	ldr	r0, =coherency_base
-	ldr	r0, [r0]
+	/* use virtual address of the coherency register */
+	ldr	r1, =coherency_base
+	ldr	r1, [r1]
 2:
-	/* Create bit by cpu index */
-	mrc	15, 0, r1, cr0, cr0, 5
-	and	r1, r1, #15
+	mov	pc, lr
+ENDPROC(ll_get_coherency_base)
+
+/* Returns with the CPU ID in r3 (r0 is untouched)*/
+ENTRY(ll_get_cpuid)
+	mrc	15, 0, r3, cr0, cr0, 5
+	and	r3, r3, #15
 	mov	r2, #(1 << 24)
-	lsl	r1, r2, r1
-	ARM_BE8(rev	r1, r1)
+	lsl	r3, r2, r3
+	ARM_BE8(rev	r3, r3)
+	mov	pc, lr
+ENDPROC(ll_get_cpuid)
 
-	/* Add CPU to SMP group - Atomic */
-	add	r3, r0, #ARMADA_XP_CFB_CTL_REG_OFFSET
-1:
-	ldrex	r2, [r3]
-	orr	r2, r2, r1
-	strex 	r0, r2, [r3]
-	cmp	r0, #0
-	bne 1b
+/* ll_add_cpu_to_smp_group, ll_enable_coherency and
+ * ll_disable_coherency use strex/ldrex whereas MMU can be off. The
+ * Armada XP SoC has an exclusive monitor that can track transactions
+ * to Device and/or SO and as such also when MMU is disabled the
+ * exclusive transactions will be functional
+ */
 
-	/* Enable coherency on CPU - Atomic */
-	add	r3, r3, #ARMADA_XP_CFB_CFG_REG_OFFSET
+ENTRY(ll_add_cpu_to_smp_group)
+	/*
+	 * r0 being untouched in ll_get_coherency_base and
+	 * ll_get_cpuid, we can use it to save lr modifing it with the
+	 * following bl
+	 */
+	mov r0, lr
+	bl	ll_get_coherency_base
+	bl	ll_get_cpuid
+	mov lr, r0
+	add	r0, r1, #ARMADA_XP_CFB_CFG_REG_OFFSET
 1:
-	ldrex	r2, [r3]
-	orr	r2, r2, r1
-	strex	r0, r2, [r3]
-	cmp	r0, #0
-	bne 1b
+	ldrex	r2, [r0]
+	orr	r2, r2, r3
+	strex	r1, r2, [r0]
+	cmp	r1, #0
+	bne	1b
+	mov	pc, lr
+ENDPROC(ll_add_cpu_to_smp_group)
 
+ENTRY(ll_enable_coherency)
+	/*
+	 * r0 being untouched in ll_get_coherency_base and
+	 * ll_get_cpuid, we can use it to save lr modifing it with the
+	 * following bl
+	 */
+	mov r0, lr
+	bl	ll_get_coherency_base
+	bl	ll_get_cpuid
+	mov lr, r0
+	add	r0, r1, #ARMADA_XP_CFB_CTL_REG_OFFSET
+1:
+	ldrex	r2, [r0]
+	orr	r2, r2, r3
+	strex	r1, r2, [r0]
+	cmp	r1, #0
+	bne	1b
 	dsb
-
 	mov	r0, #0
 	mov	pc, lr
-ENDPROC(ll_set_cpu_coherent)
+ENDPROC(ll_enable_coherency)
+
 
 	.align 2
 3:
diff --git a/arch/arm/mach-mvebu/headsmp.S b/arch/arm/mach-mvebu/headsmp.S
index cf7abe6554f7..2c4032e368ba 100644
--- a/arch/arm/mach-mvebu/headsmp.S
+++ b/arch/arm/mach-mvebu/headsmp.S
@@ -31,8 +31,10 @@
 ENTRY(armada_xp_secondary_startup)
  ARM_BE8(setend	be )			@ go BE8 if entered LE
 
-	/* Add CPU to coherency fabric */
-	bl	ll_set_cpu_coherent
+	bl	ll_add_cpu_to_smp_group
+
+	bl	ll_enable_coherency
+
 	b	secondary_startup
 
 ENDPROC(armada_xp_secondary_startup)
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v5 06/14] ARM: mvebu: Low level function to disable HW coherency support
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
                   ` (4 preceding siblings ...)
  2014-03-25 22:48 ` [PATCH v5 05/14] ARM: mvebu: Split low level functions to manipulate HW coherency Gregory CLEMENT
@ 2014-03-25 22:48 ` Gregory CLEMENT
  2014-03-25 22:48 ` [PATCH v5 07/14] ARM: mvebu: Extend the pmsu registers Gregory CLEMENT
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

When going to deep idle we need to disable the SoC snooping (aka
hardware coherency support). Playing with the coherency fabric
requires to use assembly code to be sure that the compiler doesn't
reorder the instructions nor do wrong optimization.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/mach-mvebu/coherency_ll.S | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/mach-mvebu/coherency_ll.S b/arch/arm/mach-mvebu/coherency_ll.S
index c7179a03891d..6e1b31fb1a94 100644
--- a/arch/arm/mach-mvebu/coherency_ll.S
+++ b/arch/arm/mach-mvebu/coherency_ll.S
@@ -102,6 +102,26 @@ ENTRY(ll_enable_coherency)
 	mov	pc, lr
 ENDPROC(ll_enable_coherency)
 
+ENTRY(ll_disable_coherency)
+	/*
+	 * r0 being untouched in ll_get_coherency_base and
+	 * ll_get_cpuid, we can use it to save lr modifing it with the
+	 * following bl
+	 */
+	mov r0, lr
+	bl	ll_get_coherency_base
+	bl	ll_get_cpuid
+	mov lr, r0
+	add	r0, r1, #ARMADA_XP_CFB_CTL_REG_OFFSET
+1:
+	ldrex	r2, [r0]
+	bic	r2, r2, r3
+	strex	r1, r2, [r0]
+	cmp	r1, #0
+	bne	1b
+	dsb
+	mov	pc, lr
+ENDPROC(ll_disable_coherency)
 
 	.align 2
 3:
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v5 07/14] ARM: mvebu: Extend the pmsu registers
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
                   ` (5 preceding siblings ...)
  2014-03-25 22:48 ` [PATCH v5 06/14] ARM: mvebu: Low level function to disable HW coherency support Gregory CLEMENT
@ 2014-03-25 22:48 ` Gregory CLEMENT
  2014-03-26  0:30   ` Jason Cooper
  2014-03-26 12:04   ` Sebastian Hesselbarth
  2014-03-25 22:48 ` [PATCH v5 08/14] ARM: dts: mvebu: Introduce a new compatible for the PMSU node Gregory CLEMENT
                   ` (7 subsequent siblings)
  14 siblings, 2 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

The initial binding for PMSU were wrong. It didn't take into account
all the registers from the PMSU and moreover it referred to registers
which are not part of PMSU.

The Power Management Unit Service block also controls the Coherency
Fabric subsystem. These registers are needed for the CPU idle
implementation for the Armada 370/XP, it allows to enter a deep CPU
idle state where the Coherency Fabric and the L2 cache are powered
down.

This commit add support for a new compatible for the PMSU node
including the block related to the coherency fabric. It also keeps
compatibility with the old binding

This patch also adds warnings if one of the base registers set can't
be ioremapped.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/mach-mvebu/pmsu.c | 47 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 42 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
index d71ef53107c4..865bcb651e01 100644
--- a/arch/arm/mach-mvebu/pmsu.c
+++ b/arch/arm/mach-mvebu/pmsu.c
@@ -27,11 +27,21 @@
 static void __iomem *pmsu_mp_base;
 static void __iomem *pmsu_reset_base;
 
-#define PMSU_BOOT_ADDR_REDIRECT_OFFSET(cpu)	((cpu * 0x100) + 0x24)
+#define PMSU_BASE_OFFSET    0x100
+#define PMSU_REG_SIZE	    0x1000
+
+#define PMSU_BOOT_ADDR_REDIRECT_OFFSET(cpu)	((cpu * 0x100) + 0x124)
 #define PMSU_RESET_CTL_OFFSET(cpu)		(cpu * 0x8)
 
 static struct of_device_id of_pmsu_table[] = {
-	{.compatible = "marvell,armada-370-xp-pmsu"},
+	{
+		.compatible = "marvell,armada-370-pmsu",
+		.data = (void *) false,
+	},
+	{
+		.compatible = "marvell,armada-370-xp-pmsu",
+		.data = (void *) true, /* legacy */
+	},
 	{ /* end of list */ },
 };
 
@@ -59,15 +69,42 @@ int armada_xp_boot_cpu(unsigned int cpu_id, void *boot_addr)
 }
 #endif
 
+static void __init armada_370_xp_pmsu_legacy_init(struct device_node *np)
+{
+	u32 addr;
+	pr_warn("*** Warning ***  Using an old binding which will be deprecated\n");
+	/* We just need the adress, we already know the size */
+	addr = be32_to_cpu(*of_get_address(np, 0, NULL, NULL));
+	addr -= PMSU_BASE_OFFSET;
+	pmsu_mp_base = ioremap(addr, PMSU_REG_SIZE);
+	of_node_put(np);
+}
+
 static int __init armada_370_xp_pmsu_init(void)
 {
 	struct device_node *np;
-
 	np = of_find_matching_node(NULL, of_pmsu_table);
 	if (np) {
+		const struct of_device_id *match =
+			of_match_node(of_pmsu_table, np);
+		BUG_ON(!match);
+
 		pr_info("Initializing Power Management Service Unit\n");
-		pmsu_mp_base = of_iomap(np, 0);
-		pmsu_reset_base = of_iomap(np, 1);
+
+		if (match->data) /* legacy */
+			armada_370_xp_pmsu_legacy_init(np);
+		else
+			pmsu_mp_base = of_iomap(np, 0);
+		WARN_ON(!pmsu_mp_base);
+		of_node_put(np);
+
+		/*
+		 * This temporaty hack will be removed as soon as we
+		 * get the proper reset controler support
+		 */
+		np = of_find_compatible_node(NULL, NULL, "marvell,armada-xp-cpu-reset");
+		pmsu_reset_base = of_iomap(np, 0);
+		WARN_ON(!pmsu_reset_base);
 		of_node_put(np);
 	}
 
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v5 08/14] ARM: dts: mvebu: Introduce a new compatible for the PMSU node
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
                   ` (6 preceding siblings ...)
  2014-03-25 22:48 ` [PATCH v5 07/14] ARM: mvebu: Extend the pmsu registers Gregory CLEMENT
@ 2014-03-25 22:48 ` Gregory CLEMENT
  2014-03-26  0:32   ` Jason Cooper
  2014-03-25 22:48 ` [PATCH v5 09/14] ARM: mvebu: Allow to power down L2 cache controller in idle mode Gregory CLEMENT
                   ` (6 subsequent siblings)
  14 siblings, 1 reply; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

The initial binding for PMSU were wrong. It didn't take into account
all the registers from the PMSU and in the other hand refereed to
registers which are not part of PMSU. This commit introduces a new
compatible for the PMSU node and annotates the previous one as
obsolete. However we still continue to support it during a few
releases.

Cc: devicetree at vger.kernel.org
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 .../devicetree/bindings/arm/armada-370-xp-pmsu.txt         | 14 ++++++--------
 arch/arm/boot/dts/armada-xp.dtsi                           | 11 ++++++++---
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/armada-370-xp-pmsu.txt b/Documentation/devicetree/bindings/arm/armada-370-xp-pmsu.txt
index 926b4d6aae7e..976188770079 100644
--- a/Documentation/devicetree/bindings/arm/armada-370-xp-pmsu.txt
+++ b/Documentation/devicetree/bindings/arm/armada-370-xp-pmsu.txt
@@ -4,17 +4,15 @@ Available on Marvell SOCs: Armada 370 and Armada XP
 
 Required properties:
 
-- compatible: "marvell,armada-370-xp-pmsu"
+- compatible: should be "marvell,armada-370-pmsu", whereas
+  "marvell,armada-370-xp-pmsu" is deprecated and will be removed
 
-- reg: Should contain PMSU registers location and length. First pair
-  for the per-CPU SW Reset Control registers, second pair for the
-  Power Management Service Unit.
+- reg: Should contain PMSU registers location and length.
 
 Example:
 
-armada-370-xp-pmsu at d0022000 {
-	compatible = "marvell,armada-370-xp-pmsu";
-	reg = <0xd0022100 0x430>,
-	      <0xd0020800 0x20>;
+armada-370-xp-pmsu at 22000 {
+	compatible = "marvell,armada-370-pmsu";
+	reg = <0x22000 0x1000>;
 };
 
diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
index abb9f9dcc525..0c455f5458df 100644
--- a/arch/arm/boot/dts/armada-xp.dtsi
+++ b/arch/arm/boot/dts/armada-xp.dtsi
@@ -117,9 +117,14 @@
 				clock-names = "nbclk", "fixed";
 			};
 
-			armada-370-xp-pmsu at 22000 {
-				compatible = "marvell,armada-370-xp-pmsu";
-				reg = <0x22100 0x400>, <0x20800 0x20>;
+			cpurst at 20800 {
+				compatible = "marvell,armada-xp-cpu-reset";
+				reg = <0x20800 0x20>;
+			};
+
+			armada-370-pmsu at 22000 {
+				compatible = "marvell,armada-370-pmsu";
+				reg = <0x22000 0x400>;
 			};
 
 			eth2: ethernet at 30000 {
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v5 09/14] ARM: mvebu: Allow to power down L2 cache controller in idle mode
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
                   ` (7 preceding siblings ...)
  2014-03-25 22:48 ` [PATCH v5 08/14] ARM: dts: mvebu: Introduce a new compatible for the PMSU node Gregory CLEMENT
@ 2014-03-25 22:48 ` Gregory CLEMENT
  2014-03-25 22:48 ` [PATCH v5 10/14] ARM: mvebu: Add the PMSU related part of the cpu idle functions Gregory CLEMENT
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

This commit adds a function which adjusts the PMSU configuration to
automatically power down the L2 and coherency fabric when we enter a
certain idle state.

This feature is part of the Power Management Service Unit of the
Armada 370 and Armada XP SoCs.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/mach-mvebu/pmsu.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
index 865bcb651e01..3cfb56f980f3 100644
--- a/arch/arm/mach-mvebu/pmsu.c
+++ b/arch/arm/mach-mvebu/pmsu.c
@@ -30,8 +30,15 @@ static void __iomem *pmsu_reset_base;
 #define PMSU_BASE_OFFSET    0x100
 #define PMSU_REG_SIZE	    0x1000
 
-#define PMSU_BOOT_ADDR_REDIRECT_OFFSET(cpu)	((cpu * 0x100) + 0x124)
-#define PMSU_RESET_CTL_OFFSET(cpu)		(cpu * 0x8)
+/* PMSU MP registers */
+#define PMSU_BOOT_ADDR_REDIRECT_OFFSET(cpu) ((cpu * 0x100) + 0x124)
+
+/* PMSU reset registers */
+#define PMSU_RESET_CTL_OFFSET(cpu)	    (cpu * 0x8)
+
+/* PMSU fabric registers */
+#define L2C_NFABRIC_PM_CTL		    0x4
+#define L2C_NFABRIC_PM_CTL_PWR_DOWN		BIT(20)
 
 static struct of_device_id of_pmsu_table[] = {
 	{
@@ -111,4 +118,17 @@ static int __init armada_370_xp_pmsu_init(void)
 	return 0;
 }
 
+static void armada_370_xp_pmsu_enable_l2_powerdown_onidle(void)
+{
+	u32 reg;
+
+	if (pmsu_mp_base == NULL)
+		return;
+
+	/* Enable L2 & Fabric powerdown in Deep-Idle mode - Fabric */
+	reg = readl(pmsu_mp_base + L2C_NFABRIC_PM_CTL);
+	reg |= L2C_NFABRIC_PM_CTL_PWR_DOWN;
+	writel(reg, pmsu_mp_base + L2C_NFABRIC_PM_CTL);
+}
+
 early_initcall(armada_370_xp_pmsu_init);
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v5 10/14] ARM: mvebu: Add the PMSU related part of the cpu idle functions
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
                   ` (8 preceding siblings ...)
  2014-03-25 22:48 ` [PATCH v5 09/14] ARM: mvebu: Allow to power down L2 cache controller in idle mode Gregory CLEMENT
@ 2014-03-25 22:48 ` Gregory CLEMENT
  2014-03-25 22:48 ` [PATCH v5 11/14] ARM: mvebu: Set the start address of a CPU in a separate function Gregory CLEMENT
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

The cpu idle support will need to access to Power Management Service
Unit. This commit adds the architecture related functions that will be
used in the idle path of the cpuidle driver.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/mach-mvebu/pmsu.c | 132 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 131 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
index 3cfb56f980f3..c45029f0c0b4 100644
--- a/arch/arm/mach-mvebu/pmsu.c
+++ b/arch/arm/mach-mvebu/pmsu.c
@@ -21,8 +21,11 @@
 #include <linux/of_address.h>
 #include <linux/io.h>
 #include <linux/smp.h>
+#include <asm/cacheflush.h>
+#include <asm/cp15.h>
 #include <asm/smp_plat.h>
-#include "pmsu.h"
+#include <asm/suspend.h>
+#include <asm/tlbflush.h>
 
 static void __iomem *pmsu_mp_base;
 static void __iomem *pmsu_reset_base;
@@ -31,6 +34,24 @@ static void __iomem *pmsu_reset_base;
 #define PMSU_REG_SIZE	    0x1000
 
 /* PMSU MP registers */
+#define PMSU_CONTROL_AND_CONFIG(cpu)	    ((cpu * 0x100) + 0x104)
+#define PMSU_CONTROL_AND_CONFIG_DFS_REQ		BIT(18)
+#define PMSU_CONTROL_AND_CONFIG_PWDDN_REQ	BIT(16)
+#define PMSU_CONTROL_AND_CONFIG_L2_PWDDN	BIT(20)
+
+#define PMSU_CPU_POWER_DOWN_CONTROL(cpu)    ((cpu * 0x100) + 0x108)
+
+#define PMSU_CPU_POWER_DOWN_DIS_SNP_Q_SKIP	BIT(0)
+
+#define PMSU_STATUS_AND_MASK(cpu)	    ((cpu * 0x100) + 0x10c)
+#define PMSU_STATUS_AND_MASK_CPU_IDLE_WAIT	BIT(16)
+#define PMSU_STATUS_AND_MASK_SNP_Q_EMPTY_WAIT	BIT(17)
+#define PMSU_STATUS_AND_MASK_IRQ_WAKEUP		BIT(20)
+#define PMSU_STATUS_AND_MASK_FIQ_WAKEUP		BIT(21)
+#define PMSU_STATUS_AND_MASK_DBG_WAKEUP		BIT(22)
+#define PMSU_STATUS_AND_MASK_IRQ_MASK		BIT(24)
+#define PMSU_STATUS_AND_MASK_FIQ_MASK		BIT(25)
+
 #define PMSU_BOOT_ADDR_REDIRECT_OFFSET(cpu) ((cpu * 0x100) + 0x124)
 
 /* PMSU reset registers */
@@ -40,6 +61,9 @@ static void __iomem *pmsu_reset_base;
 #define L2C_NFABRIC_PM_CTL		    0x4
 #define L2C_NFABRIC_PM_CTL_PWR_DOWN		BIT(20)
 
+extern void ll_disable_coherency(void);
+extern void ll_enable_coherency(void);
+
 static struct of_device_id of_pmsu_table[] = {
 	{
 		.compatible = "marvell,armada-370-pmsu",
@@ -131,4 +155,110 @@ static void armada_370_xp_pmsu_enable_l2_powerdown_onidle(void)
 	writel(reg, pmsu_mp_base + L2C_NFABRIC_PM_CTL);
 }
 
+static void armada_370_xp_cpu_resume(void)
+{
+	asm volatile("bl    ll_add_cpu_to_smp_group\n\t"
+		     "bl    ll_enable_coherency\n\t"
+		     "b	    cpu_resume\n\t");
+}
+
+/* No locking is needed because we only access per-CPU registers */
+void armada_370_xp_pmsu_idle_prepare(bool deepidle)
+{
+	unsigned int hw_cpu = cpu_logical_map(smp_processor_id());
+	u32 reg;
+
+	if (pmsu_mp_base == NULL)
+		return;
+
+	/*
+	 * Adjust the PMSU configuration to wait for WFI signal, enable
+	 * IRQ and FIQ as wakeup events, set wait for snoop queue empty
+	 * indication and mask IRQ and FIQ from CPU
+	 */
+	reg = readl(pmsu_mp_base + PMSU_STATUS_AND_MASK(hw_cpu));
+	reg |= PMSU_STATUS_AND_MASK_CPU_IDLE_WAIT    |
+	       PMSU_STATUS_AND_MASK_IRQ_WAKEUP       |
+	       PMSU_STATUS_AND_MASK_FIQ_WAKEUP       |
+	       PMSU_STATUS_AND_MASK_SNP_Q_EMPTY_WAIT |
+	       PMSU_STATUS_AND_MASK_IRQ_MASK         |
+	       PMSU_STATUS_AND_MASK_FIQ_MASK;
+	writel(reg, pmsu_mp_base + PMSU_STATUS_AND_MASK(hw_cpu));
+
+	reg = readl(pmsu_mp_base + PMSU_CONTROL_AND_CONFIG(hw_cpu));
+	/* ask HW to power down the L2 Cache if needed */
+	if (deepidle)
+		reg |= PMSU_CONTROL_AND_CONFIG_L2_PWDDN;
+
+	/* request power down */
+	reg |= PMSU_CONTROL_AND_CONFIG_PWDDN_REQ;
+	writel(reg, pmsu_mp_base + PMSU_CONTROL_AND_CONFIG(hw_cpu));
+
+	/* Disable snoop disable by HW - SW is taking care of it */
+	reg = readl(pmsu_mp_base + PMSU_CPU_POWER_DOWN_CONTROL(hw_cpu));
+	reg |= PMSU_CPU_POWER_DOWN_DIS_SNP_Q_SKIP;
+	writel(reg, pmsu_mp_base + PMSU_CPU_POWER_DOWN_CONTROL(hw_cpu));
+}
+
+static noinline int do_armada_370_xp_cpu_suspend(unsigned long deepidle)
+{
+	armada_370_xp_pmsu_idle_prepare(deepidle);
+
+	v7_exit_coherency_flush(all);
+
+	ll_disable_coherency();
+
+	dsb();
+
+	wfi();
+
+	/* If we are here, wfi failed. As processors run out of
+	 * coherency for some time, tlbs might be stale, so flush them
+	 */
+	local_flush_tlb_all();
+
+	ll_enable_coherency();
+
+	/* Test the CR_C bit and set it if it was cleared */
+	asm volatile(
+	"mrc	p15, 0, %0, c1, c0, 0 \n\t"
+	"tst	%0, #(1 << 2) \n\t"
+	"orreq	%0, %0, #(1 << 2) \n\t"
+	"mcreq	p15, 0, %0, c1, c0, 0 \n\t"
+	"isb	"
+	: : "r" (0));
+
+	pr_warn("Failed to suspend the system\n");
+
+	return 0;
+}
+
+static int armada_370_xp_cpu_suspend(unsigned long deepidle)
+{
+	return cpu_suspend(deepidle, do_armada_370_xp_cpu_suspend);
+}
+
+/* No locking is needed because we only access per-CPU registers */
+static noinline void armada_370_xp_pmsu_idle_restore(void)
+{
+	unsigned int hw_cpu = cpu_logical_map(smp_processor_id());
+	u32 reg;
+
+	if (pmsu_mp_base == NULL)
+		return;
+
+	/* cancel ask HW to power down the L2 Cache if possible */
+	reg = readl(pmsu_mp_base + PMSU_CONTROL_AND_CONFIG(hw_cpu));
+	reg &= ~PMSU_CONTROL_AND_CONFIG_L2_PWDDN;
+	writel(reg, pmsu_mp_base + PMSU_CONTROL_AND_CONFIG(hw_cpu));
+
+	/* cancel Enable wakeup events and mask interrupts */
+	reg = readl(pmsu_mp_base + PMSU_STATUS_AND_MASK(hw_cpu));
+	reg &= ~(PMSU_STATUS_AND_MASK_IRQ_WAKEUP | PMSU_STATUS_AND_MASK_FIQ_WAKEUP);
+	reg &= ~PMSU_STATUS_AND_MASK_CPU_IDLE_WAIT;
+	reg &= ~PMSU_STATUS_AND_MASK_SNP_Q_EMPTY_WAIT;
+	reg &= ~(PMSU_STATUS_AND_MASK_IRQ_MASK | PMSU_STATUS_AND_MASK_FIQ_MASK);
+	writel(reg, pmsu_mp_base + PMSU_STATUS_AND_MASK(hw_cpu));
+}
+
 early_initcall(armada_370_xp_pmsu_init);
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v5 11/14] ARM: mvebu: Set the start address of a CPU in a separate function
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
                   ` (9 preceding siblings ...)
  2014-03-25 22:48 ` [PATCH v5 10/14] ARM: mvebu: Add the PMSU related part of the cpu idle functions Gregory CLEMENT
@ 2014-03-25 22:48 ` Gregory CLEMENT
  2014-03-25 22:48 ` [PATCH v5 12/14] ARM: mvebu: Register notifier callback for the cpuidle transition Gregory CLEMENT
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

Setting the start (or boot) address of a CPU is no more used only
during SMP bring up, but it will also be used by the CPU idle
functions or later by the CPU hot plug ones.

This commit moves it in a separate function.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/mach-mvebu/pmsu.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
index c45029f0c0b4..3fc25d94e893 100644
--- a/arch/arm/mach-mvebu/pmsu.c
+++ b/arch/arm/mach-mvebu/pmsu.c
@@ -76,6 +76,12 @@ static struct of_device_id of_pmsu_table[] = {
 	{ /* end of list */ },
 };
 
+static void armada_370_xp_pmsu_set_start_addr(void *start_addr, int hw_cpu)
+{
+	writel(virt_to_phys(start_addr), pmsu_mp_base +
+		PMSU_BOOT_ADDR_REDIRECT_OFFSET(hw_cpu));
+}
+
 #ifdef CONFIG_SMP
 int armada_xp_boot_cpu(unsigned int cpu_id, void *boot_addr)
 {
@@ -88,8 +94,7 @@ int armada_xp_boot_cpu(unsigned int cpu_id, void *boot_addr)
 
 	hw_cpu = cpu_logical_map(cpu_id);
 
-	writel(virt_to_phys(boot_addr), pmsu_mp_base +
-			PMSU_BOOT_ADDR_REDIRECT_OFFSET(hw_cpu));
+	armada_370_xp_pmsu_set_start_addr(boot_addr, hw_cpu);
 
 	/* Release CPU from reset by clearing reset bit*/
 	reg = readl(pmsu_reset_base + PMSU_RESET_CTL_OFFSET(hw_cpu));
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v5 12/14] ARM: mvebu: Register notifier callback for the cpuidle transition
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
                   ` (10 preceding siblings ...)
  2014-03-25 22:48 ` [PATCH v5 11/14] ARM: mvebu: Set the start address of a CPU in a separate function Gregory CLEMENT
@ 2014-03-25 22:48 ` Gregory CLEMENT
  2014-03-25 22:48 ` [PATCH v5 13/14] cpuidle: mvebu: Add initial CPU idle support for Armada 370/XP SoC Gregory CLEMENT
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

In order to have well encapsulated code, we use notifier callbacks for
CPU_PM_ENTER and CPU_PM_EXIT inside the mvebu power management code.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/mach-mvebu/pmsu.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
index 3fc25d94e893..5812ff4a9b63 100644
--- a/arch/arm/mach-mvebu/pmsu.c
+++ b/arch/arm/mach-mvebu/pmsu.c
@@ -16,6 +16,7 @@
  * other SOC units
  */
 
+#include <linux/cpu_pm.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/of_address.h>
@@ -266,4 +267,22 @@ static noinline void armada_370_xp_pmsu_idle_restore(void)
 	writel(reg, pmsu_mp_base + PMSU_STATUS_AND_MASK(hw_cpu));
 }
 
+static int armada_370_xp_cpu_pm_notify(struct notifier_block *self,
+				    unsigned long action, void *hcpu)
+{
+	if (action == CPU_PM_ENTER) {
+		unsigned int hw_cpu = cpu_logical_map(smp_processor_id());
+		armada_370_xp_pmsu_set_start_addr(armada_370_xp_cpu_resume,
+						hw_cpu);
+	} else if (action == CPU_PM_EXIT) {
+		armada_370_xp_pmsu_idle_restore();
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block armada_370_xp_cpu_pm_notifier = {
+	.notifier_call = armada_370_xp_cpu_pm_notify,
+};
+
 early_initcall(armada_370_xp_pmsu_init);
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v5 13/14] cpuidle: mvebu: Add initial CPU idle support for Armada 370/XP SoC
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
                   ` (11 preceding siblings ...)
  2014-03-25 22:48 ` [PATCH v5 12/14] ARM: mvebu: Register notifier callback for the cpuidle transition Gregory CLEMENT
@ 2014-03-25 22:48 ` Gregory CLEMENT
  2014-03-25 22:48 ` [PATCH v5 14/14] ARM: mvebu: register the cpuidle driver for the Armada XP SoCs Gregory CLEMENT
  2014-03-26  0:42 ` [PATCH v5 00/14] CPU idle for Armada XP Jason Cooper
  14 siblings, 0 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

Add the wfi, cpu idle and cpu deep idle power states support for the
Armada XP SoCs.

All the latencies and the power consumption values used at the
"armada_370_xp_idle_driver" structure are preliminary and will be
modified in the future after running some measurements and analysis.

Based on the work of Nadav Haklai.

Signed-off-by: Nadav Haklai <nadavh@marvell.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/cpuidle/Kconfig.arm             |  5 ++
 drivers/cpuidle/Makefile                |  1 +
 drivers/cpuidle/cpuidle-armada-370-xp.c | 93 +++++++++++++++++++++++++++++++++
 3 files changed, 99 insertions(+)
 create mode 100644 drivers/cpuidle/cpuidle-armada-370-xp.c

diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index 97ccc31dbdd8..5bb94780d377 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -1,6 +1,11 @@
 #
 # ARM CPU Idle drivers
 #
+config ARM_ARMADA_370_XP_CPUIDLE
+	bool "CPU Idle Driver for Armada 370/XP family processors"
+	depends on ARCH_MVEBU
+	help
+	  Select this to enable cpuidle on Armada 370/XP processors.
 
 config ARM_BIG_LITTLE_CPUIDLE
 	bool "Support for ARM big.LITTLE processors"
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index f71ae1b373c5..9902d052bd87 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
 
 ##################################################################################
 # ARM SoC drivers
+obj-$(CONFIG_ARM_ARMADA_370_XP_CPUIDLE) += cpuidle-armada-370-xp.o
 obj-$(CONFIG_ARM_BIG_LITTLE_CPUIDLE)	+= cpuidle-big_little.o
 obj-$(CONFIG_ARM_HIGHBANK_CPUIDLE)	+= cpuidle-calxeda.o
 obj-$(CONFIG_ARM_KIRKWOOD_CPUIDLE)	+= cpuidle-kirkwood.o
diff --git a/drivers/cpuidle/cpuidle-armada-370-xp.c b/drivers/cpuidle/cpuidle-armada-370-xp.c
new file mode 100644
index 000000000000..6b98608c222e
--- /dev/null
+++ b/drivers/cpuidle/cpuidle-armada-370-xp.c
@@ -0,0 +1,93 @@
+/*
+ * Marvell Armada 370 and Armada XP SoC cpuidle driver
+ *
+ * Copyright (C) 2013 Marvell
+ *
+ * Nadav Haklai <nadavh@marvell.com>
+ * Gregory CLEMENT <gregory.clement@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ *
+ * Maintainer: Gregory CLEMENT <gregory.clement@free-electrons.com>
+ */
+
+#include <linux/cpu_pm.h>
+#include <linux/cpuidle.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/suspend.h>
+#include <linux/platform_device.h>
+#include <asm/cpuidle.h>
+
+#define ARMADA_370_XP_MAX_STATES	3
+#define ARMADA_370_XP_FLAG_DEEP_IDLE	0x10000
+
+static int (*armada_370_xp_cpu_suspend)(int);
+
+static int armada_370_xp_enter_idle(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv,
+				int index)
+{
+	int ret;
+	bool deepidle = false;
+	cpu_pm_enter();
+
+	if (drv->states[index].flags & ARMADA_370_XP_FLAG_DEEP_IDLE)
+		deepidle = true;
+
+	ret = armada_370_xp_cpu_suspend(deepidle);
+	if (ret)
+		return ret;
+
+	cpu_pm_exit();
+
+	return index;
+}
+
+static struct cpuidle_driver armada_370_xp_idle_driver = {
+	.name			= "armada_370_xp_idle",
+	.states[0]		= ARM_CPUIDLE_WFI_STATE,
+	.states[1]		= {
+		.enter			= armada_370_xp_enter_idle,
+		.exit_latency		= 10,
+		.power_usage		= 50,
+		.target_residency	= 100,
+		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.name			= "MV CPU IDLE",
+		.desc			= "CPU power down",
+	},
+	.states[2]		= {
+		.enter			= armada_370_xp_enter_idle,
+		.exit_latency		= 100,
+		.power_usage		= 5,
+		.target_residency	= 1000,
+		.flags			= CPUIDLE_FLAG_TIME_VALID |
+						ARMADA_370_XP_FLAG_DEEP_IDLE,
+		.name			= "MV CPU DEEP IDLE",
+		.desc			= "CPU and L2 Fabric power down",
+	},
+	.state_count = ARMADA_370_XP_MAX_STATES,
+};
+
+static int armada_370_xp_cpuidle_probe(struct platform_device *pdev)
+{
+
+	armada_370_xp_cpu_suspend = (void *)(pdev->dev.platform_data);
+	return cpuidle_register(&armada_370_xp_idle_driver, NULL);
+}
+
+static struct platform_driver armada_370_xp_cpuidle_plat_driver = {
+	.driver = {
+		.name = "cpuidle-armada-370-xp",
+		.owner = THIS_MODULE,
+	},
+	.probe = armada_370_xp_cpuidle_probe,
+};
+
+module_platform_driver(armada_370_xp_cpuidle_plat_driver);
+
+MODULE_AUTHOR("Gregory CLEMENT <gregory.clement@free-electrons.com>");
+MODULE_DESCRIPTION("Armada 370/XP cpu idle driver");
+MODULE_LICENSE("GPL");
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v5 14/14] ARM: mvebu: register the cpuidle driver for the Armada XP SoCs
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
                   ` (12 preceding siblings ...)
  2014-03-25 22:48 ` [PATCH v5 13/14] cpuidle: mvebu: Add initial CPU idle support for Armada 370/XP SoC Gregory CLEMENT
@ 2014-03-25 22:48 ` Gregory CLEMENT
  2014-03-26 10:30   ` Ezequiel Garcia
  2014-03-26 10:31   ` Thomas Petazzoni
  2014-03-26  0:42 ` [PATCH v5 00/14] CPU idle for Armada XP Jason Cooper
  14 siblings, 2 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-25 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

The cpuidle is a platform driver so we register the device just after
the initialization of the board in an arch_initcall.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/mach-mvebu/pmsu.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
index 5812ff4a9b63..fa611594b65a 100644
--- a/arch/arm/mach-mvebu/pmsu.c
+++ b/arch/arm/mach-mvebu/pmsu.c
@@ -21,6 +21,7 @@
 #include <linux/init.h>
 #include <linux/of_address.h>
 #include <linux/io.h>
+#include <linux/platform_device.h>
 #include <linux/smp.h>
 #include <asm/cacheflush.h>
 #include <asm/cp15.h>
@@ -65,6 +66,10 @@ static void __iomem *pmsu_reset_base;
 extern void ll_disable_coherency(void);
 extern void ll_enable_coherency(void);
 
+static struct platform_device armada_xp_cpuidle_device = {
+	.name = "cpuidle-armada-370-xp",
+};
+
 static struct of_device_id of_pmsu_table[] = {
 	{
 		.compatible = "marvell,armada-370-pmsu",
@@ -285,4 +290,21 @@ static struct notifier_block armada_370_xp_cpu_pm_notifier = {
 	.notifier_call = armada_370_xp_cpu_pm_notify,
 };
 
+int __init armada_370_xp_cpu_pm_init(void)
+{
+	if (!((of_find_compatible_node(NULL, NULL, "marvell,armada-370-xp-pmsu") ||
+				of_find_compatible_node(NULL, NULL, "marvell,armada-370-pmsu"))
+			&& of_find_compatible_node(NULL, NULL, "marvell,coherency-fabric")
+			&& of_machine_is_compatible("marvell,armadaxp")))
+		return 0;
+
+	armada_370_xp_pmsu_enable_l2_powerdown_onidle();
+	armada_xp_cpuidle_device.dev.platform_data = armada_370_xp_cpu_suspend;
+	platform_device_register(&armada_xp_cpuidle_device);
+	cpu_pm_register_notifier(&armada_370_xp_cpu_pm_notifier);
+
+	return 0;
+}
+
+arch_initcall(armada_370_xp_cpu_pm_init);
 early_initcall(armada_370_xp_pmsu_init);
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v5 07/14] ARM: mvebu: Extend the pmsu registers
  2014-03-25 22:48 ` [PATCH v5 07/14] ARM: mvebu: Extend the pmsu registers Gregory CLEMENT
@ 2014-03-26  0:30   ` Jason Cooper
  2014-03-26  9:33     ` Gregory CLEMENT
  2014-03-26 12:04   ` Sebastian Hesselbarth
  1 sibling, 1 reply; 29+ messages in thread
From: Jason Cooper @ 2014-03-26  0:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Mar 25, 2014 at 11:48:18PM +0100, Gregory CLEMENT wrote:
> The initial binding for PMSU were wrong. It didn't take into account
> all the registers from the PMSU and moreover it referred to registers
> which are not part of PMSU.
> 
> The Power Management Unit Service block also controls the Coherency
> Fabric subsystem. These registers are needed for the CPU idle
> implementation for the Armada 370/XP, it allows to enter a deep CPU
> idle state where the Coherency Fabric and the L2 cache are powered
> down.
> 
> This commit add support for a new compatible for the PMSU node
> including the block related to the coherency fabric. It also keeps
> compatibility with the old binding
> 
> This patch also adds warnings if one of the base registers set can't
> be ioremapped.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  arch/arm/mach-mvebu/pmsu.c | 47 +++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 42 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
> index d71ef53107c4..865bcb651e01 100644
> --- a/arch/arm/mach-mvebu/pmsu.c
> +++ b/arch/arm/mach-mvebu/pmsu.c
> @@ -27,11 +27,21 @@
>  static void __iomem *pmsu_mp_base;
>  static void __iomem *pmsu_reset_base;
>  
> -#define PMSU_BOOT_ADDR_REDIRECT_OFFSET(cpu)	((cpu * 0x100) + 0x24)
> +#define PMSU_BASE_OFFSET    0x100
> +#define PMSU_REG_SIZE	    0x1000
> +
> +#define PMSU_BOOT_ADDR_REDIRECT_OFFSET(cpu)	((cpu * 0x100) + 0x124)
>  #define PMSU_RESET_CTL_OFFSET(cpu)		(cpu * 0x8)
>  
>  static struct of_device_id of_pmsu_table[] = {
> -	{.compatible = "marvell,armada-370-xp-pmsu"},
> +	{
> +		.compatible = "marvell,armada-370-pmsu",
> +		.data = (void *) false,

This looks sketchy to me.

> +	},
> +	{
> +		.compatible = "marvell,armada-370-xp-pmsu",
> +		.data = (void *) true, /* legacy */

Same.

> +	},
>  	{ /* end of list */ },
>  };
>  
> @@ -59,15 +69,42 @@ int armada_xp_boot_cpu(unsigned int cpu_id, void *boot_addr)
>  }
>  #endif
>  
> +static void __init armada_370_xp_pmsu_legacy_init(struct device_node *np)
> +{
> +	u32 addr;
> +	pr_warn("*** Warning ***  Using an old binding which will be deprecated\n");

This should be noted in the binding docs...

> +	/* We just need the adress, we already know the size */

nit. s/adress/address/

> +	addr = be32_to_cpu(*of_get_address(np, 0, NULL, NULL));
> +	addr -= PMSU_BASE_OFFSET;
> +	pmsu_mp_base = ioremap(addr, PMSU_REG_SIZE);
> +	of_node_put(np);
> +}
> +
>  static int __init armada_370_xp_pmsu_init(void)
>  {
>  	struct device_node *np;
> -

Please leave this empty line.

>  	np = of_find_matching_node(NULL, of_pmsu_table);
>  	if (np) {
> +		const struct of_device_id *match =
> +			of_match_node(of_pmsu_table, np);
> +		BUG_ON(!match);
> +
>  		pr_info("Initializing Power Management Service Unit\n");
> -		pmsu_mp_base = of_iomap(np, 0);
> -		pmsu_reset_base = of_iomap(np, 1);
> +
> +		if (match->data) /* legacy */
> +			armada_370_xp_pmsu_legacy_init(np);

And if a new compatible string actually needs data passed?

> +		else
> +			pmsu_mp_base = of_iomap(np, 0);
> +		WARN_ON(!pmsu_mp_base);
> +		of_node_put(np);
> +
> +		/*
> +		 * This temporaty hack will be removed as soon as we

nit. s/temporaty/temporary/

thx,

Jason.

> +		 * get the proper reset controler support
> +		 */
> +		np = of_find_compatible_node(NULL, NULL, "marvell,armada-xp-cpu-reset");
> +		pmsu_reset_base = of_iomap(np, 0);
> +		WARN_ON(!pmsu_reset_base);
>  		of_node_put(np);
>  	}
>  
> -- 
> 1.8.1.2
> 

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v5 08/14] ARM: dts: mvebu: Introduce a new compatible for the PMSU node
  2014-03-25 22:48 ` [PATCH v5 08/14] ARM: dts: mvebu: Introduce a new compatible for the PMSU node Gregory CLEMENT
@ 2014-03-26  0:32   ` Jason Cooper
  0 siblings, 0 replies; 29+ messages in thread
From: Jason Cooper @ 2014-03-26  0:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Mar 25, 2014 at 11:48:19PM +0100, Gregory CLEMENT wrote:
> The initial binding for PMSU were wrong. It didn't take into account
> all the registers from the PMSU and in the other hand refereed to
> registers which are not part of PMSU. This commit introduces a new
> compatible for the PMSU node and annotates the previous one as
> obsolete. However we still continue to support it during a few
> releases.
> 
> Cc: devicetree at vger.kernel.org
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  .../devicetree/bindings/arm/armada-370-xp-pmsu.txt         | 14 ++++++--------
>  arch/arm/boot/dts/armada-xp.dtsi                           | 11 ++++++++---
>  2 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/armada-370-xp-pmsu.txt b/Documentation/devicetree/bindings/arm/armada-370-xp-pmsu.txt
> index 926b4d6aae7e..976188770079 100644
> --- a/Documentation/devicetree/bindings/arm/armada-370-xp-pmsu.txt
> +++ b/Documentation/devicetree/bindings/arm/armada-370-xp-pmsu.txt
> @@ -4,17 +4,15 @@ Available on Marvell SOCs: Armada 370 and Armada XP
>  
>  Required properties:
>  
> -- compatible: "marvell,armada-370-xp-pmsu"
> +- compatible: should be "marvell,armada-370-pmsu", whereas
> +  "marvell,armada-370-xp-pmsu" is deprecated and will be removed

:)  ignore my comment on the previous patch.

> -- reg: Should contain PMSU registers location and length. First pair
> -  for the per-CPU SW Reset Control registers, second pair for the
> -  Power Management Service Unit.
> +- reg: Should contain PMSU registers location and length.
>  
>  Example:
>  
> -armada-370-xp-pmsu at d0022000 {
> -	compatible = "marvell,armada-370-xp-pmsu";
> -	reg = <0xd0022100 0x430>,
> -	      <0xd0020800 0x20>;
> +armada-370-xp-pmsu at 22000 {
> +	compatible = "marvell,armada-370-pmsu";
> +	reg = <0x22000 0x1000>;
>  };
>  
> diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
> index abb9f9dcc525..0c455f5458df 100644
> --- a/arch/arm/boot/dts/armada-xp.dtsi
> +++ b/arch/arm/boot/dts/armada-xp.dtsi
> @@ -117,9 +117,14 @@
>  				clock-names = "nbclk", "fixed";
>  			};
>  
> -			armada-370-xp-pmsu at 22000 {
> -				compatible = "marvell,armada-370-xp-pmsu";
> -				reg = <0x22100 0x400>, <0x20800 0x20>;
> +			cpurst at 20800 {
> +				compatible = "marvell,armada-xp-cpu-reset";
> +				reg = <0x20800 0x20>;
> +			};
> +
> +			armada-370-pmsu at 22000 {
> +				compatible = "marvell,armada-370-pmsu";
> +				reg = <0x22000 0x400>;
>  			};
>  
>  			eth2: ethernet at 30000 {
> -- 
> 1.8.1.2
> 

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v5 00/14] CPU idle for Armada XP
  2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
                   ` (13 preceding siblings ...)
  2014-03-25 22:48 ` [PATCH v5 14/14] ARM: mvebu: register the cpuidle driver for the Armada XP SoCs Gregory CLEMENT
@ 2014-03-26  0:42 ` Jason Cooper
  2014-03-26  9:53   ` Gregory CLEMENT
  14 siblings, 1 reply; 29+ messages in thread
From: Jason Cooper @ 2014-03-26  0:42 UTC (permalink / raw)
  To: linux-arm-kernel

Gregory,

On Tue, Mar 25, 2014 at 11:48:11PM +0100, Gregory CLEMENT wrote:
...
> The first patch should go through ARM subsystem and should be taken by
> Russell King. I made few change on it following Lorenzo advice and
> now it will reuse the cpu v7 suspend and resume function and just do
> specific operation before calling them.
> 
> The 13th patch 'cpuidle: mvebu: Add initial cpu idle support for
> Armada 370/XP SoC' is the only one who should go to the cpuidle
> subsystem. But of course I would like that Daniel Lezcano or Rafael
> J. Wysocki have a look on the whole series and especially patches 10,
> 12 and 14.

Ok, I'm tired.  :)  Help me out here.  Patches 1 and 13 are depended on
by the other patches and vice-versa, right?  In that case, I'm thinking
it's best to seek Acks from the other maintainers and keep the whole
branch together.  Please tell me if I'm wrong.

thx,

Jason.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v5 07/14] ARM: mvebu: Extend the pmsu registers
  2014-03-26  0:30   ` Jason Cooper
@ 2014-03-26  9:33     ` Gregory CLEMENT
  2014-03-26  9:55       ` Sebastian Hesselbarth
  0 siblings, 1 reply; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-26  9:33 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Jason,

On 26/03/2014 01:30, Jason Cooper wrote:
> On Tue, Mar 25, 2014 at 11:48:18PM +0100, Gregory CLEMENT wrote:
>> The initial binding for PMSU were wrong. It didn't take into account
>> all the registers from the PMSU and moreover it referred to registers
>> which are not part of PMSU.
>>
>> The Power Management Unit Service block also controls the Coherency
>> Fabric subsystem. These registers are needed for the CPU idle
>> implementation for the Armada 370/XP, it allows to enter a deep CPU
>> idle state where the Coherency Fabric and the L2 cache are powered
>> down.
>>
>> This commit add support for a new compatible for the PMSU node
>> including the block related to the coherency fabric. It also keeps
>> compatibility with the old binding
>>
>> This patch also adds warnings if one of the base registers set can't
>> be ioremapped.
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>>  arch/arm/mach-mvebu/pmsu.c | 47 +++++++++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 42 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
>> index d71ef53107c4..865bcb651e01 100644
>> --- a/arch/arm/mach-mvebu/pmsu.c
>> +++ b/arch/arm/mach-mvebu/pmsu.c
>> @@ -27,11 +27,21 @@
>>  static void __iomem *pmsu_mp_base;
>>  static void __iomem *pmsu_reset_base;
>>  
>> -#define PMSU_BOOT_ADDR_REDIRECT_OFFSET(cpu)	((cpu * 0x100) + 0x24)
>> +#define PMSU_BASE_OFFSET    0x100
>> +#define PMSU_REG_SIZE	    0x1000
>> +
>> +#define PMSU_BOOT_ADDR_REDIRECT_OFFSET(cpu)	((cpu * 0x100) + 0x124)
>>  #define PMSU_RESET_CTL_OFFSET(cpu)		(cpu * 0x8)
>>  
>>  static struct of_device_id of_pmsu_table[] = {
>> -	{.compatible = "marvell,armada-370-xp-pmsu"},
>> +	{
>> +		.compatible = "marvell,armada-370-pmsu",
>> +		.data = (void *) false,
> 
> This looks sketchy to me.

Could you elaborate it?

For a boolean I didn't saw the point to use a pointer.

> 
>> +	},
>> +	{
>> +		.compatible = "marvell,armada-370-xp-pmsu",
>> +		.data = (void *) true, /* legacy */
> 
> Same.
> 
>> +	},
>>  	{ /* end of list */ },
>>  };
>>  
>> @@ -59,15 +69,42 @@ int armada_xp_boot_cpu(unsigned int cpu_id, void *boot_addr)
>>  }
>>  #endif
>>  
>> +static void __init armada_370_xp_pmsu_legacy_init(struct device_node *np)
>> +{
>> +	u32 addr;
>> +	pr_warn("*** Warning ***  Using an old binding which will be deprecated\n");
> 
> This should be noted in the binding docs...
> 
>> +	/* We just need the adress, we already know the size */
> 
> nit. s/adress/address/

Thanks

> 
>> +	addr = be32_to_cpu(*of_get_address(np, 0, NULL, NULL));
>> +	addr -= PMSU_BASE_OFFSET;
>> +	pmsu_mp_base = ioremap(addr, PMSU_REG_SIZE);
>> +	of_node_put(np);
>> +}
>> +
>>  static int __init armada_370_xp_pmsu_init(void)
>>  {
>>  	struct device_node *np;
>> -
> 
> Please leave this empty line.
> 

OK


>>  	np = of_find_matching_node(NULL, of_pmsu_table);
>>  	if (np) {
>> +		const struct of_device_id *match =
>> +			of_match_node(of_pmsu_table, np);
>> +		BUG_ON(!match);
>> +
>>  		pr_info("Initializing Power Management Service Unit\n");
>> -		pmsu_mp_base = of_iomap(np, 0);
>> -		pmsu_reset_base = of_iomap(np, 1);
>> +
>> +		if (match->data) /* legacy */
>> +			armada_370_xp_pmsu_legacy_init(np);
> 
> And if a new compatible string actually needs data passed?

in this case we would have to update the of_pmsu_table, so this
code could be also updated if needed in the same time. So I don't
see the problem, maybe I miss something.

But the plan is really to remove this legacy part later (after a
few kernel release)

> 
>> +		else
>> +			pmsu_mp_base = of_iomap(np, 0);
>> +		WARN_ON(!pmsu_mp_base);
>> +		of_node_put(np);
>> +
>> +		/*
>> +		 * This temporaty hack will be removed as soon as we
> 
> nit. s/temporaty/temporary/

Thanks

> 
> thx,
> 
> Jason.
> 
>> +		 * get the proper reset controler support
>> +		 */
>> +		np = of_find_compatible_node(NULL, NULL, "marvell,armada-xp-cpu-reset");
>> +		pmsu_reset_base = of_iomap(np, 0);
>> +		WARN_ON(!pmsu_reset_base);
>>  		of_node_put(np);
>>  	}
>>  
>> -- 
>> 1.8.1.2
>>


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v5 00/14] CPU idle for Armada XP
  2014-03-26  0:42 ` [PATCH v5 00/14] CPU idle for Armada XP Jason Cooper
@ 2014-03-26  9:53   ` Gregory CLEMENT
  0 siblings, 0 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-26  9:53 UTC (permalink / raw)
  To: linux-arm-kernel

On 26/03/2014 01:42, Jason Cooper wrote:
> Gregory,
> 
> On Tue, Mar 25, 2014 at 11:48:11PM +0100, Gregory CLEMENT wrote:
> ...
>> The first patch should go through ARM subsystem and should be taken by
>> Russell King. I made few change on it following Lorenzo advice and
>> now it will reuse the cpu v7 suspend and resume function and just do
>> specific operation before calling them.
>>
>> The 13th patch 'cpuidle: mvebu: Add initial cpu idle support for
>> Armada 370/XP SoC' is the only one who should go to the cpuidle
>> subsystem. But of course I would like that Daniel Lezcano or Rafael
>> J. Wysocki have a look on the whole series and especially patches 10,
>> 12 and 14.
> 
> Ok, I'm tired.  :)  Help me out here.  Patches 1 and 13 are depended on
> by the other patches and vice-versa, right?  In that case, I'm thinking
> it's best to seek Acks from the other maintainers and keep the whole
> branch together.  Please tell me if I'm wrong.
> 

Actually for the build there is no dependencies between patch 1, 13 and the
other ones.

About running the kernel obviously you need the full series to have the CPU
idle working, but if patch 13 is not applied or if it is applied alone, the
kernel won't crash, the only consequence will be that the cpu idle driver won't
be loaded.

If all patches are applied except patch 1 then we could experiment some issues.
However, now the the amount of code is very small in this patch, I planed
to submit it to Russell King's patch system soon.

About patch 13, I agree that if Daniel and Rafael agree to give their acked-by,
it will be easier to handle if you can take it in a single series, but it is not
mandatory.

Thanks,

Gregory


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v5 07/14] ARM: mvebu: Extend the pmsu registers
  2014-03-26  9:33     ` Gregory CLEMENT
@ 2014-03-26  9:55       ` Sebastian Hesselbarth
  0 siblings, 0 replies; 29+ messages in thread
From: Sebastian Hesselbarth @ 2014-03-26  9:55 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/26/2014 10:33 AM, Gregory CLEMENT wrote:
> On 26/03/2014 01:30, Jason Cooper wrote:
>> On Tue, Mar 25, 2014 at 11:48:18PM +0100, Gregory CLEMENT wrote:
>>> The initial binding for PMSU were wrong. It didn't take into account
>>> all the registers from the PMSU and moreover it referred to registers
>>> which are not part of PMSU.
>>>
>>> The Power Management Unit Service block also controls the Coherency
>>> Fabric subsystem. These registers are needed for the CPU idle
>>> implementation for the Armada 370/XP, it allows to enter a deep CPU
>>> idle state where the Coherency Fabric and the L2 cache are powered
>>> down.
>>>
>>> This commit add support for a new compatible for the PMSU node
>>> including the block related to the coherency fabric. It also keeps
>>> compatibility with the old binding
>>>
>>> This patch also adds warnings if one of the base registers set can't
>>> be ioremapped.
>>>
>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>> ---
>>>   arch/arm/mach-mvebu/pmsu.c | 47 +++++++++++++++++++++++++++++++++++++++++-----
>>>   1 file changed, 42 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
>>> index d71ef53107c4..865bcb651e01 100644
>>> --- a/arch/arm/mach-mvebu/pmsu.c
>>> +++ b/arch/arm/mach-mvebu/pmsu.c
>>> @@ -27,11 +27,21 @@
>>>   static void __iomem *pmsu_mp_base;
>>>   static void __iomem *pmsu_reset_base;
>>>
>>> -#define PMSU_BOOT_ADDR_REDIRECT_OFFSET(cpu)	((cpu * 0x100) + 0x24)
>>> +#define PMSU_BASE_OFFSET    0x100
>>> +#define PMSU_REG_SIZE	    0x1000
>>> +
>>> +#define PMSU_BOOT_ADDR_REDIRECT_OFFSET(cpu)	((cpu * 0x100) + 0x124)
>>>   #define PMSU_RESET_CTL_OFFSET(cpu)		(cpu * 0x8)
>>>
>>>   static struct of_device_id of_pmsu_table[] = {
>>> -	{.compatible = "marvell,armada-370-xp-pmsu"},
>>> +	{
>>> +		.compatible = "marvell,armada-370-pmsu",
>>> +		.data = (void *) false,
>>
>> This looks sketchy to me.
>
> Could you elaborate it?
>
> For a boolean I didn't saw the point to use a pointer.

Isn't the different compatible boolean enough?
You can use of_device_is_compatible() below.

>>> +	},
>>> +	{
>>> +		.compatible = "marvell,armada-370-xp-pmsu",
>>> +		.data = (void *) true, /* legacy */
>>
>> Same.
>>
>>> +	},
>>>   	{ /* end of list */ },
>>>   };
>>>
>>> @@ -59,15 +69,42 @@ int armada_xp_boot_cpu(unsigned int cpu_id, void *boot_addr)
>>>   }
>>>   #endif
>>>
>>> +static void __init armada_370_xp_pmsu_legacy_init(struct device_node *np)
>>> +{
>>> +	u32 addr;
>>> +	pr_warn("*** Warning ***  Using an old binding which will be deprecated\n");
>>
>> This should be noted in the binding docs...

pr_warn(FW_{WARN,BUG} "deprecated pmsu binding\n");

[...]
>>>   	np = of_find_matching_node(NULL, of_pmsu_table);
>>>   	if (np) {
>>> +		const struct of_device_id *match =
>>> +			of_match_node(of_pmsu_table, np);
>>> +		BUG_ON(!match);
>>> +
>>>   		pr_info("Initializing Power Management Service Unit\n");
>>> -		pmsu_mp_base = of_iomap(np, 0);
>>> -		pmsu_reset_base = of_iomap(np, 1);
>>> +
>>> +		if (match->data) /* legacy */
>>> +			armada_370_xp_pmsu_legacy_init(np);

if (of_device_is_compatible(np, "marvell,armada-370-xp-pmsu"))
	armada_370_xp_pmsu_legacy_init(np);
else
	...

Sebastian

>> And if a new compatible string actually needs data passed?
>
> in this case we would have to update the of_pmsu_table, so this
> code could be also updated if needed in the same time. So I don't
> see the problem, maybe I miss something.
>
> But the plan is really to remove this legacy part later (after a
> few kernel release)
>
>>
>>> +		else
>>> +			pmsu_mp_base = of_iomap(np, 0);
>>> +		WARN_ON(!pmsu_mp_base);
>>> +		of_node_put(np);
>>> +
>>> +		/*
>>> +		 * This temporaty hack will be removed as soon as we

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v5 14/14] ARM: mvebu: register the cpuidle driver for the Armada XP SoCs
  2014-03-25 22:48 ` [PATCH v5 14/14] ARM: mvebu: register the cpuidle driver for the Armada XP SoCs Gregory CLEMENT
@ 2014-03-26 10:30   ` Ezequiel Garcia
  2014-03-26 10:31   ` Thomas Petazzoni
  1 sibling, 0 replies; 29+ messages in thread
From: Ezequiel Garcia @ 2014-03-26 10:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Gregory,

On Mar 25, Gregory CLEMENT wrote:
> @@ -285,4 +290,21 @@ static struct notifier_block armada_370_xp_cpu_pm_notifier = {
>  	.notifier_call = armada_370_xp_cpu_pm_notify,
>  };
>  
> +int __init armada_370_xp_cpu_pm_init(void)
> +{
> +	if (!((of_find_compatible_node(NULL, NULL, "marvell,armada-370-xp-pmsu") ||
> +				of_find_compatible_node(NULL, NULL, "marvell,armada-370-pmsu"))
> +			&& of_find_compatible_node(NULL, NULL, "marvell,coherency-fabric")
> +			&& of_machine_is_compatible("marvell,armadaxp")))
> +		return 0;
> +

of_find_compatible_node() gets the returned node, so you should put it
after using it. How about something along this pseudo-code?

pmsu = of_find_compatible_node(pmsu);
coherency_fabric = of_find_compatible_node(coherency_fabric);

if (pmsu && coherency_fabric && of_machine_is_compatible(armadaxp)) {
	/* Do your thing ... */
}

of_node_put(pmsu);
of_node_put(coherency_fabric);

And you also gain some readability, as a side effect.
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v5 14/14] ARM: mvebu: register the cpuidle driver for the Armada XP SoCs
  2014-03-25 22:48 ` [PATCH v5 14/14] ARM: mvebu: register the cpuidle driver for the Armada XP SoCs Gregory CLEMENT
  2014-03-26 10:30   ` Ezequiel Garcia
@ 2014-03-26 10:31   ` Thomas Petazzoni
  2014-03-26 10:38     ` Gregory CLEMENT
  1 sibling, 1 reply; 29+ messages in thread
From: Thomas Petazzoni @ 2014-03-26 10:31 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Gregory CLEMENT,

On Tue, 25 Mar 2014 23:48:25 +0100, Gregory CLEMENT wrote:

> +int __init armada_370_xp_cpu_pm_init(void)
> +{
> +	if (!((of_find_compatible_node(NULL, NULL, "marvell,armada-370-xp-pmsu") ||
> +				of_find_compatible_node(NULL, NULL, "marvell,armada-370-pmsu"))
> +			&& of_find_compatible_node(NULL, NULL, "marvell,coherency-fabric")
> +			&& of_machine_is_compatible("marvell,armadaxp")))
> +		return 0;

Instead of this big single condition, maybe it could be split in a
nicer way:

	/*
	 * Check that all the requirements are available to enable
	 * cpuidle. So far, it is only supported on Armada XP, cpuidle
	 * needs the coherency fabric and the PMSU enabled
         */

	if (!of_machine_is_compatible("marvell,armadaxp"))
		return 0;

	np = of_find_compatible_node(NULL, NULL, "marvell,coherency-fabric");
	if (!np)
		return 0;
	of_node_put(np);

	np = of_find_matching_node(NULL, of_pmsu_table);
	if (!np)
		return 0;
	of_node_put(np);

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v5 14/14] ARM: mvebu: register the cpuidle driver for the Armada XP SoCs
  2014-03-26 10:31   ` Thomas Petazzoni
@ 2014-03-26 10:38     ` Gregory CLEMENT
  0 siblings, 0 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-26 10:38 UTC (permalink / raw)
  To: linux-arm-kernel

On 26/03/2014 11:31, Thomas Petazzoni wrote:
> Dear Gregory CLEMENT,
> 
> On Tue, 25 Mar 2014 23:48:25 +0100, Gregory CLEMENT wrote:
> 
>> +int __init armada_370_xp_cpu_pm_init(void)
>> +{
>> +	if (!((of_find_compatible_node(NULL, NULL, "marvell,armada-370-xp-pmsu") ||
>> +				of_find_compatible_node(NULL, NULL, "marvell,armada-370-pmsu"))
>> +			&& of_find_compatible_node(NULL, NULL, "marvell,coherency-fabric")
>> +			&& of_machine_is_compatible("marvell,armadaxp")))
>> +		return 0;
> 
> Instead of this big single condition, maybe it could be split in a
> nicer way:
> 
> 	/*
> 	 * Check that all the requirements are available to enable
> 	 * cpuidle. So far, it is only supported on Armada XP, cpuidle
> 	 * needs the coherency fabric and the PMSU enabled
>          */
> 
> 	if (!of_machine_is_compatible("marvell,armadaxp"))
> 		return 0;
> 
> 	np = of_find_compatible_node(NULL, NULL, "marvell,coherency-fabric");
> 	if (!np)
> 		return 0;
> 	of_node_put(np);
> 
> 	np = of_find_matching_node(NULL, of_pmsu_table);

Oh yes the pmsu table! I was concerned by this big condition, yesterday but
I didn't find a proper solution due to the test to the 2 comaptible string:
I forgot the pmsu table.

I will do this change (I mean using the pmsu table but also splitting the
condition in small chunk).

Thanks,

Gregory


> 	if (!np)
> 		return 0;
> 	of_node_put(np);
> 
> Thomas
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v5 03/14] ARM: mvebu: ll_set_cpu_coherent always uses the current CPU
  2014-03-26 11:52   ` Sebastian Hesselbarth
@ 2014-03-26 11:51     ` Gregory CLEMENT
  0 siblings, 0 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-26 11:51 UTC (permalink / raw)
  To: linux-arm-kernel

On 26/03/2014 12:52, Sebastian Hesselbarth wrote:
> On 03/25/2014 11:48 PM, Gregory CLEMENT wrote:
>> ll_set_cpu_coherent is always used on the current CPU, so instead of
>> passing the CPU id as argument, ll_set_cpu_coherent() can find it by
>> itself.
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> ---
> [...]
>> diff --git a/arch/arm/mach-mvebu/coherency_ll.S b/arch/arm/mach-mvebu/coherency_ll.S
>> index 1f2bcd4b5424..6cb26b919787 100644
>> --- a/arch/arm/mach-mvebu/coherency_ll.S
>> +++ b/arch/arm/mach-mvebu/coherency_ll.S
>> @@ -24,9 +24,7 @@
>>   #include <asm/cp15.h>
>>
>>   	.text
>> -/*
>> - * r0: HW CPU id
>> - */
>> +
>>   ENTRY(ll_set_cpu_coherent)
>>   	mrc	p15, 0, r1, c1, c0, 0
>>   	tst	r1, #CR_M @ Check MMU bit enabled
>> @@ -43,9 +41,11 @@ ENTRY(ll_set_cpu_coherent)
>>   	ldr	r0, [r0]
>>   2:
>>   	/* Create bit by cpu index */
>> -	mov	r3, #(1 << 24)
>> -	lsl	r1, r3, r1
>> -ARM_BE8(rev	r1, r1)
>> +	mrc	15, 0, r1, cr0, cr0, 5
>> +	and	r1, r1, #15
>> +	mov	r2, #(1 << 24)
>> +	lsl	r1, r2, r1
>> +	ARM_BE8(rev	r1, r1)
> 
> nit: re-indent ARM_BE8 to the beginning of the line?

Good catch!

Thanks,

Gregory




> Sebastian
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v5 03/14] ARM: mvebu: ll_set_cpu_coherent always uses the current CPU
  2014-03-25 22:48 ` [PATCH v5 03/14] ARM: mvebu: ll_set_cpu_coherent always uses the current CPU Gregory CLEMENT
@ 2014-03-26 11:52   ` Sebastian Hesselbarth
  2014-03-26 11:51     ` Gregory CLEMENT
  0 siblings, 1 reply; 29+ messages in thread
From: Sebastian Hesselbarth @ 2014-03-26 11:52 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/25/2014 11:48 PM, Gregory CLEMENT wrote:
> ll_set_cpu_coherent is always used on the current CPU, so instead of
> passing the CPU id as argument, ll_set_cpu_coherent() can find it by
> itself.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
[...]
> diff --git a/arch/arm/mach-mvebu/coherency_ll.S b/arch/arm/mach-mvebu/coherency_ll.S
> index 1f2bcd4b5424..6cb26b919787 100644
> --- a/arch/arm/mach-mvebu/coherency_ll.S
> +++ b/arch/arm/mach-mvebu/coherency_ll.S
> @@ -24,9 +24,7 @@
>   #include <asm/cp15.h>
>
>   	.text
> -/*
> - * r0: HW CPU id
> - */
> +
>   ENTRY(ll_set_cpu_coherent)
>   	mrc	p15, 0, r1, c1, c0, 0
>   	tst	r1, #CR_M @ Check MMU bit enabled
> @@ -43,9 +41,11 @@ ENTRY(ll_set_cpu_coherent)
>   	ldr	r0, [r0]
>   2:
>   	/* Create bit by cpu index */
> -	mov	r3, #(1 << 24)
> -	lsl	r1, r3, r1
> -ARM_BE8(rev	r1, r1)
> +	mrc	15, 0, r1, cr0, cr0, 5
> +	and	r1, r1, #15
> +	mov	r2, #(1 << 24)
> +	lsl	r1, r2, r1
> +	ARM_BE8(rev	r1, r1)

nit: re-indent ARM_BE8 to the beginning of the line?

Sebastian

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v5 07/14] ARM: mvebu: Extend the pmsu registers
  2014-03-26 12:04   ` Sebastian Hesselbarth
@ 2014-03-26 12:01     ` Gregory CLEMENT
  2014-03-26 12:59     ` Thomas Petazzoni
  1 sibling, 0 replies; 29+ messages in thread
From: Gregory CLEMENT @ 2014-03-26 12:01 UTC (permalink / raw)
  To: linux-arm-kernel

On 26/03/2014 13:04, Sebastian Hesselbarth wrote:
> On 03/25/2014 11:48 PM, Gregory CLEMENT wrote:
>> The initial binding for PMSU were wrong. It didn't take into account
>> all the registers from the PMSU and moreover it referred to registers
>> which are not part of PMSU.
>>
>> The Power Management Unit Service block also controls the Coherency
>> Fabric subsystem. These registers are needed for the CPU idle
>> implementation for the Armada 370/XP, it allows to enter a deep CPU
>> idle state where the Coherency Fabric and the L2 cache are powered
>> down.
>>
>> This commit add support for a new compatible for the PMSU node
>> including the block related to the coherency fabric. It also keeps
>> compatibility with the old binding
>>
>> This patch also adds warnings if one of the base registers set can't
>> be ioremapped.
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>>   arch/arm/mach-mvebu/pmsu.c | 47 +++++++++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 42 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
>> index d71ef53107c4..865bcb651e01 100644
>> --- a/arch/arm/mach-mvebu/pmsu.c
>> +++ b/arch/arm/mach-mvebu/pmsu.c
> [...]
>> +static void __init armada_370_xp_pmsu_legacy_init(struct device_node *np)
>> +{
>> +	u32 addr;
>> +	pr_warn("*** Warning ***  Using an old binding which will be deprecated\n");
>> +	/* We just need the adress, we already know the size */
>> +	addr = be32_to_cpu(*of_get_address(np, 0, NULL, NULL));
>> +	addr -= PMSU_BASE_OFFSET;
>> +	pmsu_mp_base = ioremap(addr, PMSU_REG_SIZE);
>> +	of_node_put(np);
> 
> When messing with OF passed resources, I suggest to use
> of_address_to_resource() and work on the struct resource
> instead. The be32_to_cpu pointing to an immediate just looks
> strange.

Actually it was taken from of_address_to_resource()! As I didn't need
the size I have just took the usefull part for my need.

> 
> Sebastian
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v5 07/14] ARM: mvebu: Extend the pmsu registers
  2014-03-25 22:48 ` [PATCH v5 07/14] ARM: mvebu: Extend the pmsu registers Gregory CLEMENT
  2014-03-26  0:30   ` Jason Cooper
@ 2014-03-26 12:04   ` Sebastian Hesselbarth
  2014-03-26 12:01     ` Gregory CLEMENT
  2014-03-26 12:59     ` Thomas Petazzoni
  1 sibling, 2 replies; 29+ messages in thread
From: Sebastian Hesselbarth @ 2014-03-26 12:04 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/25/2014 11:48 PM, Gregory CLEMENT wrote:
> The initial binding for PMSU were wrong. It didn't take into account
> all the registers from the PMSU and moreover it referred to registers
> which are not part of PMSU.
>
> The Power Management Unit Service block also controls the Coherency
> Fabric subsystem. These registers are needed for the CPU idle
> implementation for the Armada 370/XP, it allows to enter a deep CPU
> idle state where the Coherency Fabric and the L2 cache are powered
> down.
>
> This commit add support for a new compatible for the PMSU node
> including the block related to the coherency fabric. It also keeps
> compatibility with the old binding
>
> This patch also adds warnings if one of the base registers set can't
> be ioremapped.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>   arch/arm/mach-mvebu/pmsu.c | 47 +++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
> index d71ef53107c4..865bcb651e01 100644
> --- a/arch/arm/mach-mvebu/pmsu.c
> +++ b/arch/arm/mach-mvebu/pmsu.c
[...]
> +static void __init armada_370_xp_pmsu_legacy_init(struct device_node *np)
> +{
> +	u32 addr;
> +	pr_warn("*** Warning ***  Using an old binding which will be deprecated\n");
> +	/* We just need the adress, we already know the size */
> +	addr = be32_to_cpu(*of_get_address(np, 0, NULL, NULL));
> +	addr -= PMSU_BASE_OFFSET;
> +	pmsu_mp_base = ioremap(addr, PMSU_REG_SIZE);
> +	of_node_put(np);

When messing with OF passed resources, I suggest to use
of_address_to_resource() and work on the struct resource
instead. The be32_to_cpu pointing to an immediate just looks
strange.

Sebastian

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v5 07/14] ARM: mvebu: Extend the pmsu registers
  2014-03-26 12:04   ` Sebastian Hesselbarth
  2014-03-26 12:01     ` Gregory CLEMENT
@ 2014-03-26 12:59     ` Thomas Petazzoni
  1 sibling, 0 replies; 29+ messages in thread
From: Thomas Petazzoni @ 2014-03-26 12:59 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Sebastian Hesselbarth,

On Wed, 26 Mar 2014 13:04:55 +0100, Sebastian Hesselbarth wrote:
> On 03/25/2014 11:48 PM, Gregory CLEMENT wrote:
> > The initial binding for PMSU were wrong. It didn't take into account
> > all the registers from the PMSU and moreover it referred to registers
> > which are not part of PMSU.
> >
> > The Power Management Unit Service block also controls the Coherency
> > Fabric subsystem. These registers are needed for the CPU idle
> > implementation for the Armada 370/XP, it allows to enter a deep CPU
> > idle state where the Coherency Fabric and the L2 cache are powered
> > down.
> >
> > This commit add support for a new compatible for the PMSU node
> > including the block related to the coherency fabric. It also keeps
> > compatibility with the old binding
> >
> > This patch also adds warnings if one of the base registers set can't
> > be ioremapped.
> >
> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> > ---
> >   arch/arm/mach-mvebu/pmsu.c | 47 +++++++++++++++++++++++++++++++++++++++++-----
> >   1 file changed, 42 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
> > index d71ef53107c4..865bcb651e01 100644
> > --- a/arch/arm/mach-mvebu/pmsu.c
> > +++ b/arch/arm/mach-mvebu/pmsu.c
> [...]
> > +static void __init armada_370_xp_pmsu_legacy_init(struct device_node *np)
> > +{
> > +	u32 addr;
> > +	pr_warn("*** Warning ***  Using an old binding which will be deprecated\n");
> > +	/* We just need the adress, we already know the size */
> > +	addr = be32_to_cpu(*of_get_address(np, 0, NULL, NULL));
> > +	addr -= PMSU_BASE_OFFSET;
> > +	pmsu_mp_base = ioremap(addr, PMSU_REG_SIZE);
> > +	of_node_put(np);
> 
> When messing with OF passed resources, I suggest to use
> of_address_to_resource() and work on the struct resource
> instead. The be32_to_cpu pointing to an immediate just looks
> strange.

I'm working on a revised patch set for these PMSU changes, as I also
need them as a preparation for the SMP support on Armada 375/38x. And
my revised patch set makes the manipulations on the 'struct resource',
as you suggest.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2014-03-26 12:59 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-25 22:48 [PATCH v5 00/14] CPU idle for Armada XP Gregory CLEMENT
2014-03-25 22:48 ` [PATCH v5 01/14] ARM: PJ4B: Add cpu_suspend/cpu_resume hooks for PJ4B Gregory CLEMENT
2014-03-25 22:48 ` [PATCH v5 02/14] ARM: mvebu: remove the address parameter for ll_set_cpu_coherent Gregory CLEMENT
2014-03-25 22:48 ` [PATCH v5 03/14] ARM: mvebu: ll_set_cpu_coherent always uses the current CPU Gregory CLEMENT
2014-03-26 11:52   ` Sebastian Hesselbarth
2014-03-26 11:51     ` Gregory CLEMENT
2014-03-25 22:48 ` [PATCH v5 04/14] ARM: mvebu: Remove the unused argument of set_cpu_coherent() Gregory CLEMENT
2014-03-25 22:48 ` [PATCH v5 05/14] ARM: mvebu: Split low level functions to manipulate HW coherency Gregory CLEMENT
2014-03-25 22:48 ` [PATCH v5 06/14] ARM: mvebu: Low level function to disable HW coherency support Gregory CLEMENT
2014-03-25 22:48 ` [PATCH v5 07/14] ARM: mvebu: Extend the pmsu registers Gregory CLEMENT
2014-03-26  0:30   ` Jason Cooper
2014-03-26  9:33     ` Gregory CLEMENT
2014-03-26  9:55       ` Sebastian Hesselbarth
2014-03-26 12:04   ` Sebastian Hesselbarth
2014-03-26 12:01     ` Gregory CLEMENT
2014-03-26 12:59     ` Thomas Petazzoni
2014-03-25 22:48 ` [PATCH v5 08/14] ARM: dts: mvebu: Introduce a new compatible for the PMSU node Gregory CLEMENT
2014-03-26  0:32   ` Jason Cooper
2014-03-25 22:48 ` [PATCH v5 09/14] ARM: mvebu: Allow to power down L2 cache controller in idle mode Gregory CLEMENT
2014-03-25 22:48 ` [PATCH v5 10/14] ARM: mvebu: Add the PMSU related part of the cpu idle functions Gregory CLEMENT
2014-03-25 22:48 ` [PATCH v5 11/14] ARM: mvebu: Set the start address of a CPU in a separate function Gregory CLEMENT
2014-03-25 22:48 ` [PATCH v5 12/14] ARM: mvebu: Register notifier callback for the cpuidle transition Gregory CLEMENT
2014-03-25 22:48 ` [PATCH v5 13/14] cpuidle: mvebu: Add initial CPU idle support for Armada 370/XP SoC Gregory CLEMENT
2014-03-25 22:48 ` [PATCH v5 14/14] ARM: mvebu: register the cpuidle driver for the Armada XP SoCs Gregory CLEMENT
2014-03-26 10:30   ` Ezequiel Garcia
2014-03-26 10:31   ` Thomas Petazzoni
2014-03-26 10:38     ` Gregory CLEMENT
2014-03-26  0:42 ` [PATCH v5 00/14] CPU idle for Armada XP Jason Cooper
2014-03-26  9:53   ` Gregory CLEMENT

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).