CPU Idle for Armada 370 and Armada 38x

Sorting the headers in alphabetic order will help to reduce the conflict when adding new headers later.

Signed-off-by Gregory CLEMENT <[email protected]>

---
 arch/arm/mach-mvebu/pmsu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

arch/arm/mach-mvebu/pmsu.c +0 -0

index a1d407c0febe..5584d35b8e88 100644

arch/arm/mach-mvebu/pmsu.c line 24

Quoted 6 lines

#include <linux/cpu_pm.h>

arch/arm/mach-mvebu/pmsu.c +1 -2

-#include <linux/kernel.h>
 #include <linux/init.h>
-#include <linux/of_address.h>
+#include <linux/kernel.h>
 #include <linux/io.h>

So, with your patch applied, if I'm correct, we will have:

#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/io.h>

It seems like we're not yet alphabetically sorted here :)

On some of the mvebu SoC and due to internal bootrom issue, CPU

initial jump code should be placed in SRAM memory of the SoC. In order

to achieve this, we have to unmap the BootROM and at some specific location where the BootROM was place, create a specific MBus window

for the SRAM. This SRAM is initialized with a few instructions of code that allows to jump into the real secondary CPU boot address.

This work around currently needed for booting SMP on Aramda 375 Z1 and

will be needed for cpuidle support on Armada 370. Instead of duplicate

the same code, this commit introduce a common function to handle it:

mvebu_boot_addr_wa().

Signed-off-by Gregory CLEMENT <[email protected]>

---
 arch/arm/mach-mvebu/pmsu.c    | 31 +++++++++++++++++++++++++++++++
 arch/arm/mach-mvebu/pmsu.h    |  1 +
 arch/arm/mach-mvebu/pmsu_ll.S | 19 +++++++++++++++++++
 3 files changed, 51 insertions(+)

arch/arm/mach-mvebu/pmsu.c +1 -0

index 5584d35b8e88..991560905ccc 100644
@@ -22,6 +22,7 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/io.h>
+#include <linux/mbus.h>
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/resource.h>

arch/arm/mach-mvebu/pmsu.c +5 -0

+#define SRAM_PHYS_BASE	    0xFFFF0000
+
+#define BOOTROM_BASE    0xFFF00000
+#define BOOTROM_SIZE    0x100000
+
 extern void ll_disable_coherency(void);
 extern void ll_enable_coherency(void);
 

arch/arm/mach-mvebu/pmsu.c +2 -0

+}
+
 static int __init armada_370_xp_pmsu_init(void)
 {
 	struct device_node *np;

arch/arm/mach-mvebu/pmsu.h +1 -0

index 07a737c6b95d..2d97a70c2558 100644
@@ -12,5 +12,6 @@
 #define __MACH_MVEBU_PMSU_H
 
 int armada_xp_boot_cpu(unsigned int cpu_id, void *phys_addr);
+void mvebu_boot_addr_wa(int crypto_eng_id, unsigned long resume_addr_reg);
 
 #endif	/* __MACH_370_XP_PMSU_H */

arch/arm/mach-mvebu/pmsu_ll.S +0 -0

index fc3de68d8c54..3b702a16bd3d 100644

Quoted 2 lines

mvebu_boot_addr_wa().

I'm not sure the name of the function is appropriate, as we don't know what it is doing. Maybe mvebu_setup_boot_addr_wa() ?

Also, maybe your commit log should indicate that the workaround involves using the Crypto engine SRAM, which will help understanding the reference to the crypto engine in the code.

Quoted 6 lines

Signed-off-by Gregory CLEMENT <[email protected]>

---
 arch/arm/mach-mvebu/pmsu.c    | 31 +++++++++++++++++++++++++++++++
 arch/arm/mach-mvebu/pmsu.h    |  1 +
 arch/arm/mach-mvebu/pmsu_ll.S | 19 +++++++++++++++++++
 3 files changed, 51 insertions(+)

I don't really have a better suggestion, but the workaround doesn't seem to be related to the PMSU, so are pmsu.c and pmsu_ll.S really the right files to store this code?

Quoted 17 lines

arch/arm/mach-mvebu/pmsu.c +1 -0

index 5584d35b8e88..991560905ccc 100644
@@ -22,6 +22,7 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/io.h>
+#include <linux/mbus.h>
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/resource.h>

@@ -63,6 +64,14 @@ static void __iomem *pmsu_mp_base;
 #define L2C_NFABRIC_PM_CTL		    0x4
 #define L2C_NFABRIC_PM_CTL_PWR_DOWN		BIT(20)
 

arch/arm/mach-mvebu/pmsu.c static void __iomem *pmsu_mp_base; · line 67

Quoted 1 line

arch/arm/mach-mvebu/pmsu.c +1 -0

+#define ARMADA_370_CRYPT0_ENG_ID	0x9

Not needed in this file, the MBus window target ID is passed as argument to the mvebu_boot_addr_wa() function.

arch/arm/mach-mvebu/pmsu.c static void __iomem *pmsu_mp_base; · line 68

Quoted 2 lines

arch/arm/mach-mvebu/pmsu.c +1 -0

+#define CRYPT0_ENG_ATTR	0x1

For consistency, I'd prefer to see this being passed as argument to mvebu_boot_addr_wa().

Quoted 13 lines

arch/arm/mach-mvebu/pmsu.c +5 -0

+#define SRAM_PHYS_BASE	    0xFFFF0000
+
+#define BOOTROM_BASE    0xFFF00000
+#define BOOTROM_SIZE    0x100000
+
 extern void ll_disable_coherency(void);
 extern void ll_enable_coherency(void);
 

@@ -85,6 +94,28 @@ void mvebu_pmsu_set_cpu_boot_addr(int hw_cpu, void *boot_addr)
 		PMSU_BOOT_ADDR_REDIRECT_OFFSET(hw_cpu));
 }
 

arch/arm/mach-mvebu/pmsu.c void mvebu_pmsu_set_cpu_boot_addr(int hw_cpu, void *boot_addr) · line 100

Quoted 4 lines

arch/arm/mach-mvebu/pmsu.c +4 -0

+extern unsigned char mvebu_boot_wa_start;
+extern unsigned char mvebu_boot_wa_end;
+
+void mvebu_boot_addr_wa(int crypto_eng_id, u32 resume_addr_reg)

unsigned int crypto_eng_target ('unsigned int' is the type used by the mvebu-mbus API), and add unsigned int crypto_eng_attribute, to match the mvebu-mbus API. Also, void * seems more appropriate than u32 for an address, maybe even void __iomem * since it's actually pointing to a memory-mapped register.

arch/arm/mach-mvebu/pmsu.c void mvebu_pmsu_set_cpu_boot_addr(int hw_cpu, void *boot_addr) · line 108

Quoted 9 lines

arch/arm/mach-mvebu/pmsu.c +8 -0

+{
+	void __iomem *sram_virt_base;
+	u32 code_len = &mvebu_boot_wa_end - &mvebu_boot_wa_start;
+
+	mvebu_mbus_del_window(BOOTROM_BASE, BOOTROM_SIZE);
+	mvebu_mbus_add_window_by_id(crypto_eng_id, CRYPT0_ENG_ATTR,
+				SRAM_PHYS_BASE, SZ_64K);
+	sram_virt_base = ioremap(SRAM_PHYS_BASE, SZ_64K);

Maybe a return value check?

arch/arm/mach-mvebu/pmsu.c void mvebu_pmsu_set_cpu_boot_addr(int hw_cpu, void *boot_addr) · line 110

Quoted 3 lines

arch/arm/mach-mvebu/pmsu.c +2 -0

+
+

One too many new line.

arch/arm/mach-mvebu/pmsu.c void mvebu_pmsu_set_cpu_boot_addr(int hw_cpu, void *boot_addr) · line 116

Quoted 6 lines

arch/arm/mach-mvebu/pmsu.c +6 -0

+	memcpy(sram_virt_base, &mvebu_boot_wa_start, code_len);
+	/*
+	 * The last word of the code copied in SRAM must contain the
+	 * physical base address of the PMSU register
+	 */
+	*(unsigned long *)(sram_virt_base + code_len - 4) = resume_addr_reg;

Maybe instead:

	writel(resume_addr_reg, sram_virt_base + code_len - 4);

Quoted 4 lines

Quoted 1 line

+#define CRYPT0_ENG_ATTR 0x1

For consistency, I'd prefer to see this being passed as argument to mvebu_boot_addr_wa().

The attribute is the same, so why bother with it? If later we have a SoC where this attribute can be different then I agree to add this argument.

(quoted diff) +10 -0

+extern unsigned char mvebu_boot_wa_start;
+extern unsigned char mvebu_boot_wa_end;
+
+void mvebu_boot_addr_wa(int crypto_eng_id, u32 resume_addr_reg)
+{
+	void __iomem *sram_virt_base;
+	u32 code_len = &mvebu_boot_wa_end - &mvebu_boot_wa_start;
+
+	mvebu_mbus_del_window(BOOTROM_BASE, BOOTROM_SIZE);
+	mvebu_mbus_add_window_by_id(crypto_eng_id, CRYPT0_ENG_ATTR,

+.global mvebu_boot_wa_start
+.global mvebu_boot_wa_end

(quoted diff) +7 -0

+
+/* The following code will be executed from SRAM */
+ENTRY(mvebu_boot_wa_start)
+mvebu_boot_wa_start:
+/* use physical address of the boot address register register */
+	adr     r0, 1f
+	ldr     r0, [r0]

Review

Quoted 5 lines

(quoted diff) +5 -0

+	/*
+	 * The last word of the code copied in SRAM must contain the
+	 * physical base address of the PMSU register
+	 */
+	*(unsigned long *)(sram_virt_base + code_len - 4) = resume_addr_reg;

Contrary to what I said, use __raw_writel() and not writel() here, to keep the native endianness of the system when writing the value:

arch/arm/mach-mvebu/pmsu_ll.S +7 -4

@@ -43,11 +43,14 @@ ENDPROC(armada_38x_cpu_resume)
 /* The following code will be executed from SRAM */
 ENTRY(mvebu_boot_wa_start)
 mvebu_boot_wa_start:
-/* use physical address of the boot address register register */
+ARM_BE8(setend be )                    @ go BE8 if entered LE
        adr     r0, 1f
-       ldr     r0, [r0]
-       ldr     r0, [r0]
-       mov     pc, r0
+       ldr     r0, [r0]                @ load the address of the
+                                       @ resume register
+       ldr     r0, [r0]                @ load the value in the
+                                       @ resume register
+ARM_BE8(rev    r0, r0)                 @ the value is stored LE
+       mov     pc, r0                  @ jump to this value
 

Note that the first ldr r0, [r0] does not need a rev r0, r0 because the value is stored in the native endianness of the system thanks to the __raw_writel() mentioned before. However, the second ldr r0, [r0] reads the value in the Resume Address register, which is written in little-endian by the writel() call in mvebu_pmsu_set_cpu_boot_addr().

Could you include this in your next iteration of the patches?

In order to boot the secondary CPUs on Armada 375 Z1, we need to read the boot address of these CPUs through a register part of the System Controller. This done very early where MMU is not enable yet, so we need to get the physical address of this register to use it.

In the previous version of this workaround the physical address were hardcoded whereas depending of the Mbus configuration this address may change.

This commit will allow to use the generic boot workaround function with the correct address.

---
 arch/arm/mach-mvebu/common.h            |  1 +
 arch/arm/mach-mvebu/system-controller.c | 11 +++++++++++
 2 files changed, 12 insertions(+)

arch/arm/mach-mvebu/common.h +1 -0

index b67fb7a10d8b..6ad62cf8352c 100644
@@ -20,6 +20,7 @@
 void mvebu_restart(enum reboot_mode mode, const char *cmd);
 int mvebu_cpu_reset_deassert(int cpu);
 void mvebu_pmsu_set_cpu_boot_addr(int hw_cpu, void *boot_addr);
+u32 mvebu_system_controller_get_phys_addr(void);
 void mvebu_system_controller_set_cpu_boot_addr(void *boot_addr);
 
 void armada_xp_cpu_die(unsigned int cpu);

arch/arm/mach-mvebu/system-controller.c +11 -0

index 0c5524ac75b7..ae6ab543aa36 100644
@@ -30,6 +30,7 @@
 #include "common.h"
 
 static void __iomem *system_controller_base;
+static u32 system_controller_phys_base;
 
 struct mvebu_system_controller {
 	u32 rstoutn_mask_offset;

@@ -109,6 +110,13 @@ void mvebu_system_controller_set_cpu_boot_addr(void *boot_addr)
 	writel(virt_to_phys(boot_addr), system_controller_base +
 	       mvebu_sc->resume_boot_addr);
 }
+
+u32 mvebu_system_controller_get_phys_addr(void)
+{
+	BUG_ON(system_controller_phys_base == NULL);
+	BUG_ON(mvebu_sc->resume_boot_addr == 0);
+	return system_controller_phys_base + mvebu_sc->resume_boot_addr;
+}
 #endif
 
 static int __init mvebu_system_controller_init(void)

@@ -119,7 +127,10 @@ static int __init mvebu_system_controller_init(void)
 	np = of_find_matching_node_and_match(NULL, of_system_controller_table,
 					     &match);
 	if (np) {
+		struct resource res;
 		system_controller_base = of_iomap(np, 0);
+		of_address_to_resource(np, 0, &res);
+		system_controller_phys_base = res.start;
 		mvebu_sc = (struct mvebu_system_controller *)match->data;
 		of_node_put(np);
 	}

In order to boot the secondary CPUs on Armada 375 Z1, we need to read the boot address of these CPUs through a register part of the System Controller. This done very early where MMU is not enable yet, so we need to get the physical address of this register to use it.

In the previous version of this workaround the physical address were

hardcoded whereas depending of the Mbus configuration this address may change.

This commit will allow to use the generic boot workaround function with the correct address.

---
 arch/arm/mach-mvebu/common.h            |  1 +
 arch/arm/mach-mvebu/system-controller.c | 11 +++++++++++
 2 files changed, 12 insertions(+)

arch/arm/mach-mvebu/common.h +1 -0

index b67fb7a10d8b..6ad62cf8352c 100644
@@ -20,6 +20,7 @@
 void mvebu_restart(enum reboot_mode mode, const char *cmd);
 int mvebu_cpu_reset_deassert(int cpu);
 void mvebu_pmsu_set_cpu_boot_addr(int hw_cpu, void *boot_addr);
+u32 mvebu_system_controller_get_phys_addr(void);
 void mvebu_system_controller_set_cpu_boot_addr(void *boot_addr);
 
 void armada_xp_cpu_die(unsigned int cpu);

arch/arm/mach-mvebu/system-controller.c +1 -0

index 0c5524ac75b7..ae6ab543aa36 100644
@@ -30,6 +30,7 @@
 #include "common.h"
 
 static void __iomem *system_controller_base;
+static u32 system_controller_phys_base;
 
 struct mvebu_system_controller {
 	u32 rstoutn_mask_offset;

Gregory,

I'm not sure that the best approach to solve this problem. Instead, maybe the system-controller.c code should set up the boot address workaround on Armada 375. Since the workaround on 375 is really related to setting the boot address which is done by the system controller, maybe the initialization of the workaround belongs in system-controller.c ?

Quoted 6 lines

I'm not sure that the best approach to solve this problem. Instead, maybe the system-controller.c code should set up the boot address workaround on Armada 375. Since the workaround on 375 is really related to setting the boot address which is done by the system controller, maybe the initialization of the workaround belongs in system-controller.c ?

So moving the workaround into mvebu_system_controller_set_cpu_boot_addr() should be the thing to do.

Dear Gregory CLEMENT,

Quoted 7 lines · Gregory CLEMENT

Quoted 6 lines

I'm not sure that the best approach to solve this problem. Instead, maybe the system-controller.c code should set up the boot address workaround on Armada 375. Since the workaround on 375 is really related to setting the boot address which is done by the system controller, maybe the initialization of the workaround belongs in system-controller.c ?

Quoted 2 lines

So moving the workaround into mvebu_system_controller_set_cpu_boot_addr() should be the thing to do.

Except that mvebu_system_controller_set_cpu_boot_addr() is called multiple times during the system execution, while you want the workaround to be initialized once. And we surely don't really want a static variable in there to test if the workaround has already been initialized or not.

Dear Gregory CLEMENT,

Quoted 3 lines · Gregory CLEMENT

static void __iomem *system_controller_base;
+static u32 system_controller_phys_base;

arch/arm/mach-mvebu/system-controller.c void mvebu_system_controller_set_cpu_boot_addr(void *boot_addr) · line 116

Quoted 10 lines

struct mvebu_system_controller {

arch/arm/mach-mvebu/system-controller.c +4 -0

 	u32 rstoutn_mask_offset;

@@ -109,6 +110,13 @@ void mvebu_system_controller_set_cpu_boot_addr(void *boot_addr)
 	writel(virt_to_phys(boot_addr), system_controller_base +
 	       mvebu_sc->resume_boot_addr);
 }
+
+u32 mvebu_system_controller_get_phys_addr(void)
+{
+	BUG_ON(system_controller_phys_base == NULL);

This rightfully generates a fairly strong warning at compile time:
you're comparing an u32 to NULL. It should be == 0 here.

On Armada 370, there is "a slow exit process from the deep idle state due to heavy L1/L2 cache cleanup operations performed by the BootROM software" (cf errata GL-BootROM-10). To avoid this, we replace the restart code of the BootROM by a simple jump to the boot address. Then the code located at this boot address will take care of the initialization.

For this purpose, we use the common function mvebu_boot_addr_wa() introduced in the previous commit.

arch/arm/mach-mvebu/pmsu.c +9 -0

+static void __iomem *pmsu_mp_base;
+
 static struct platform_device armada_xp_cpuidle_device = {
 	.name = "cpuidle-armada-370-xp",
 };

@@ -147,6 +149,8 @@ static int __init armada_370_xp_pmsu_init(void)
 		goto out;
 	}
 
+	pmsu_mp_phys_base = res.start;
+
 	pmsu_mp_base = ioremap(res.start, resource_size(&res));
 	if (!pmsu_mp_base) {
 		pr_err("unable to map registers\n");

@@ -312,6 +316,18 @@ int __init armada_370_xp_cpu_pm_init(void)
 		return 0;
 	of_node_put(np);
 
+	/*
+	 * On Armada 370, there is "a slow exit process from the deep
+	 * idle state due to heavy L1/L2 cache cleanup operations
+	 * performed by the BootROM software". To avoid this, we
+	 * replace the restart code of the bootrom by a a simple jump

Dear Gregory CLEMENT,

Quoted 7 lines · Gregory CLEMENT

On Armada 370, there is "a slow exit process from the deep idle state due to heavy L1/L2 cache cleanup operations performed by the BootROM software" (cf errata GL-BootROM-10). To avoid this, we replace the restart code of the BootROM by a simple jump to the boot address. Then the code located at this boot address will take care of the initialization.

Quoted 2 lines

For this purpose, we use the common function mvebu_boot_addr_wa() introduced in the previous commit.

"in the previous commit" would make sense if the commit immediately before this one in the series was the one you would be referencing. But that's not the case here, so it should be either: "introduced in one of the previous commits", or better "introduced in commit <title of the commit>".

arch/arm/mach-mvebu/pmsu.c extern void ll_enable_coherency(void); · line 79

Quoted 8 lines

#define PMSU_BASE_OFFSET 0x100
#define PMSU_REG_SIZE 0x1000

arch/arm/mach-mvebu/pmsu.c +1 -0

@@ -77,6 +76,9 @@ extern void ll_enable_coherency(void);
 
 extern void armada_370_xp_cpu_resume(void);
 
+static unsigned long pmsu_mp_phys_base;

phys_addr_t.

Actually most of the function related to the PMSU are not specific to the Armada 370 or Armada XP SoCs, but can also be used for most of the other mvebu ARMv7 SoCs.

Signed-off-by Gregory CLEMENT <[email protected]>

---
 arch/arm/mach-mvebu/pmsu.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

Dear Gregory CLEMENT,

Quoted 8 lines · Gregory CLEMENT

Actually most of the function related to the PMSU are not specific to the Armada 370 or Armada XP SoCs, but can also be used for most of the other mvebu ARMv7 SoCs.

Signed-off-by Gregory CLEMENT <[email protected]>

---
 arch/arm/mach-mvebu/pmsu.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

I'm fine on the principle, but this badly conflicts with the PMSU changes I've made to support CPU hotplug, which are already in linux-next. See
http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/log/arch/arm/mach-mvebu/pmsu.c.

arch/arm/mach-mvebu/pmsu.c static void armada_370_xp_pmsu_enable_l2_powerdown_onidle(void) · line 181

Quoted 3 lines

/* No locking is needed because we only access per-CPU registers */

arch/arm/mach-mvebu/pmsu.c +1 -1

-void armada_370_xp_pmsu_idle_prepare(bool deepidle)
+static void mvebu_v7_pmsu_idle_prepare(bool deepidle)

Also, note here that you're not only renaming, but also adding the 'static' qualifier. This is already done in linux-next by
http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/arch/arm/mach-mvebu/pmsu.c?id=adb1d99384c7480886153a97d2ea22e9c0d2e053,
but the function is anyway renamed in
http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/arch/arm/mach-mvebu/pmsu.c?id=bbb92284b6c821e9434223d437fbd10b8a24c294
as a preparation for CPU hotplug support.

In order to support more mvebu SoCs, this patch use an initialization specific function associated to each SoCs which support CPU Idle.

Then each SoC will have his own set of check and of data configuration.

Signed-off-by Gregory CLEMENT <[email protected]>

---
 arch/arm/mach-mvebu/pmsu.c | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

arch/arm/mach-mvebu/pmsu.c +20 -0

index 087157c20b8a..454f0f9ede6b 100644
@@ -293,23 +293,47 @@ static struct notifier_block mvebu_v7_cpu_pm_notifier = {
 	.notifier_call = mvebu_v7_cpu_pm_notify,
 };
 
+static bool (*mvebu_v7_cpu_idle_init)(void);
+
+static __init bool armada_xp_cpuidle_init(void)
+{
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, "marvell,coherency-fabric");
+	if (!np)
+		return false;
+	of_node_put(np);
+
+	mvebu_v7_cpuidle_device.dev.platform_data = armada_xp_370_cpu_suspend;
+	return true;
+}
+
+static struct of_device_id of_cpuidle_table[] __initdata = {
+	{ .compatible = "marvell,armadaxp",
+	  .data = (void *)armada_xp_cpuidle_init,
+	},
+	{ /* end of list */ },

arch/arm/mach-mvebu/pmsu.c +4 -8

 	/*
 	 * 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
+	 * cpuidle. Each SoCs comes with its own requirements and
+	 * configuration
 	 */
 
-	if (!of_machine_is_compatible("marvell,armadaxp"))
-		return 0;
+	mvebu_v7_cpu_idle_init = (bool (*)(void))match->data;
 
-	np = of_find_compatible_node(NULL, NULL, "marvell,coherency-fabric");
-	if (!np)
+	if (!mvebu_v7_cpu_idle_init())
 		return 0;
-	of_node_put(np);
 
 	np = of_find_matching_node(NULL, of_pmsu_table);
 	if (!np)

@@ -329,7 +353,6 @@ static int __init mvebu_v7_cpu_pm_init(void)
 				PMSU_BOOT_ADDR_REDIRECT_OFFSET(0));
 
 	mvebu_v7_pmsu_enable_l2_powerdown_onidle();
-	mvebu_v7_cpuidle_device.dev.platform_data = armada_xp_370_cpu_suspend;
 	platform_device_register(&mvebu_v7_cpuidle_device);
 	cpu_pm_register_notifier(&mvebu_v7_cpu_pm_notifier);
 

Quoted 39 lines · Gregory CLEMENT

In order to support more mvebu SoCs, this patch use an initialization specific function associated to each SoCs which support CPU Idle.

Then each SoC will have his own set of check and of data configuration.

Signed-off-by Gregory CLEMENT <[email protected]>

---
 arch/arm/mach-mvebu/pmsu.c | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

arch/arm/mach-mvebu/pmsu.c +20 -0

index 087157c20b8a..454f0f9ede6b 100644
@@ -293,23 +293,47 @@ static struct notifier_block mvebu_v7_cpu_pm_notifier = {
 	.notifier_call = mvebu_v7_cpu_pm_notify,
 };
 
+static bool (*mvebu_v7_cpu_idle_init)(void);
+
+static __init bool armada_xp_cpuidle_init(void)
+{
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, "marvell,coherency-fabric");
+	if (!np)
+		return false;
+	of_node_put(np);
+
+	mvebu_v7_cpuidle_device.dev.platform_data = armada_xp_370_cpu_suspend;
+	return true;
+}
+
+static struct of_device_id of_cpuidle_table[] __initdata = {
+	{ .compatible = "marvell,armadaxp",
+	  .data = (void *)armada_xp_cpuidle_init,
+	},
+	{ /* end of list */ },

arch/arm/mach-mvebu/pmsu.c static struct notifier_block mvebu_v7_cpu_pm_notifier = { · line 325

Quoted 10 lines

arch/arm/mach-mvebu/pmsu.c +7 -0

+};
+
 static int __init mvebu_v7_cpu_pm_init(void)
 {
 	struct device_node *np;
+	const struct of_device_id *match;
+
+	np = of_find_matching_node_and_match(NULL, of_cpuidle_table,
+					&match);
+

The following part was missing (without this the kernel hang on Armada 375)

Review

Quoted 8 lines

arch/arm/mach-mvebu/pmsu.c static struct notifier_block mvebu_v7_cpu_pm_notifier = { · line 325

Quoted 3 lines

arch/arm/mach-mvebu/pmsu.c +3 -0

+	np = of_find_matching_node_and_match(NULL, of_cpuidle_table,
+					&match);
+

The following part was missing (without this the kernel hang on Armada 375)

(quoted diff) +2 -0

+       if (!np)
+               return 0;

If there's no other changes, I can fix this up when I apply it. As well as the S-o-b.

arch/arm/mach-mvebu/pmsu.c +19 -0

+static bool (*mvebu_v7_cpu_idle_init)(void);
+
+static __init bool armada_xp_cpuidle_init(void)
+{
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, "marvell,coherency-fabric");
+	if (!np)
+		return false;
+	of_node_put(np);
+
+	mvebu_v7_cpuidle_device.dev.platform_data = armada_xp_370_cpu_suspend;
+	return true;
+}
+
+static struct of_device_id of_cpuidle_table[] __initdata = {
+	{ .compatible = "marvell,armadaxp",
+	  .data = (void *)armada_xp_cpuidle_init,
+	},

arch/arm/mach-mvebu/pmsu.c static struct notifier_block mvebu_v7_cpu_pm_notifier = { · line 324

Quoted 4 lines

arch/arm/mach-mvebu/pmsu.c +4 -0

+	const struct of_device_id *match;
+
+	np = of_find_matching_node_and_match(NULL, of_cpuidle_table,
+					&match);

I'm not sure I like using of_find_matching_node_and_match() on the entire tree to find the root node compatible string. Wouldn't it be simpler and shorter to just do:

	if (of_machine_is_compatible("marvell,armadaxp"))
		ret = armadaxp_cpuidle_init();
	else if (of_machine_is_compatible("marvell,armada370"))
		ret = armada370_cpuidle_init();
	else if (of_machine_is_compaitble("marvell,armada380"))
		ret = armada38x_cpuidle_init();

	if (ret)
		return ret;

Also, using a boolean to return a success/error status is not the kernel way of doing things, you should use an int instead and use proper error codes or return 0 on success.

Dear Gregory CLEMENT,

Quoted 10 lines · Gregory CLEMENT

arch/arm/mach-mvebu/pmsu.c +2 -1

+static void *mvebu_cpu_resume;
+
 static struct platform_device mvebu_v7_cpuidle_device = {
 	.name = "cpuidle-armada-370-xp",
 };

@@ -281,7 +283,7 @@ static int mvebu_v7_cpu_pm_notify(struct notifier_block *self,
 {
 	if (action == CPU_PM_ENTER) {
 		unsigned int hw_cpu = cpu_logical_map(smp_processor_id());
-		mvebu_pmsu_set_cpu_boot_addr(hw_cpu, armada_370_xp_cpu_resume);

arch/arm/mach-mvebu/pmsu.c static int mvebu_v7_cpu_pm_notify(struct notifier_block *self, · line 286

Quoted 1 line

arch/arm/mach-mvebu/pmsu.c +1 -0

+		mvebu_pmsu_set_cpu_boot_addr(hw_cpu, mvebu_cpu_resume);

Instead of doing this in the CPU_PM_ENTER notifier, do we have a reason to not do it in the do_armada_370_xp_cpu_suspend() function, and then do_armada_38x_suspend() function respectively? Those functions are already SoC-specific, so surely, they already know whether the resume path should go through armada_370_xp_cpu_resume() or armada_38x_cpu_resume(), no?

arch/arm/mach-mvebu/pmsu.c static void mvebu_v7_pmsu_enable_l2_powerdown_onidle(void) · line 183

Quoted 3 lines

/* No locking is needed because we only access per-CPU registers */

arch/arm/mach-mvebu/pmsu.c +1 -1

-static void mvebu_v7_pmsu_idle_prepare(bool deepidle)
+static void mvebu_v7_pmsu_idle_prepare(bool deepidle, bool snoopdis)

I continue to find it odd to have more and more boolean arguments to a function to indicate what the function should do. It often indicates that the function should rather be split in smaller, fine-grained functions. Another sad consequence of using booleans is that the call sites of the functions can no longer be understood properly:

	mvebu_v7_pmsu_idle_prepare(false, true);
	mvebu_v7_pmsu_idle_prepare(true, true);
	mvebu_v7_pmsu_idle_prepare(false, false);

But oh well, it looks like it's the easiest solution here, and I admit it's convenient to have one function to the entire business of preparing for idle.

Maybe we can use flags, hence we use only one arguments, it became extensible, and more readable too. Something like

#define PMSU_IDLE_NO_FLAG 0
#define PMSU_IDLE_DEEPIDLE BIT(0)
#define PMSU_IDLE_SNOOPDIS BIT(1)

then we can have:
mvebu_v7_pmsu_idle_prepare(PMSU_IDLE_SNOOPDIS); mvebu_v7_pmsu_idle_prepare(PMSU_IDLE_DEEPIDLE | PMSU_IDLE_SNOOPDIS); mvebu_v7_pmsu_idle_prepare(PMSU_IDLE_NO_FLAG);

static void mvebu_v7_pmsu_idle_prepare(u32 flags)

The CA9 MPcore SoC Control block allows to do some configuration that the SoC could use for a specific use case. In most cases the defaults case is enough. However there is few exception: for cpuidle we need to use the CA9 MPcore Reset Control register.

Signed-off-by Gregory CLEMENT <[email protected]>

Cc devicetree at vger.kernel.org

---
 .../devicetree/bindings/arm/armada-380-mpcore-soc-ctrl.txt | 14 ++++++++++++++
 arch/arm/boot/dts/armada-38x.dtsi                          |  5 +++++
 2 files changed, 19 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/armada-380-mpcore-soc-ctrl.txt

Documentation/devicetree/bindings/arm/armada-380-mpcore-soc-ctrl.txt new file +0 -0

new file mode 100644
index 000000000000..0a2df51ba560

Documentation/devicetree/bindings/arm/armada-380-mpcore-soc-ctrl.txt new file +6 -0

+  datasheet for the CA9 MPcore SoC Control registers
+
+mpcore-soc-ctrl at 20d20 {
+	compatible = "marvell,armada-380-mpcore-soc-ctrl";
+	reg = <0x20d20 0x6c>;
+};

arch/arm/boot/dts/armada-38x.dtsi +0 -0

index 689fa1a46728..242d0ecc99f3 100644
@@ -286,6 +286,11 @@
 				reg = <0x20800 0x10>;

Quoted 4 lines

The CA9 MPcore SoC Control block allows to do some configuration that the SoC could use for a specific use case. In most cases the defaults case is enough. However there is few exception: for cpuidle we need to use the CA9 MPcore Reset Control register.

I'd reword this to something like:

"""
The CA9 MPcore SoC Control block is a set of registers that allows to configure certain internal aspects of the core blocks of the SoC (Cortex-A9, L2 cache controller, etc.). In most cases, the default values are fine so they aren't many reasons to touch those registers, but there is one exception: to support cpuidle on Armada 38x, we need to modify the value of the CA9 MPcore Reset Control register.

Therefore, this commit adds a new Device Tree binding for this hardware block, and uses this new binding for the Armada 38x Device Tree file.

"""

Quoted 1 line

+- reg: should be register base and length as documented in the

should be *the* register base and length

With those fixed:

Reviewed-by Thomas Petazzoni <[email protected]>

Actually this driver will be able to manage the cpuidle for more SoCs that Armada 370 ad XP. It will support Armada 38x and potentially Armada 375. This patch change the names accordingly to this behavior.

Quoted 3 lines

Actually this driver will be able to manage the cpuidle for more SoCs that Armada 370 ad XP. It will support Armada 38x and potentially Armada 375. This patch change the names accordingly to this behavior.

I think the last sentence should rather be something like: "This patch renames the driver as well as the functions and variables used in the driver".

I would also specifically mention the renaming of the driver that requires changing the pmsu.c file. It is worth mentioning that this rename, touching both drivers/cpuidle and arch/arm/mach-mvebu, may require some special handling in terms of patch merging (the patch mainly touches drivers/cpuidle so it should theoretically go through the cpuidle maintainer, but since the pmsu.c file is touched a lot by other patches, there will probably be lots of conflicts if this patch goes through the cpuidle tree). Surely something to mention as a comment in the patch, maybe to get the Acked-by of the cpuidle people and merge things through the mvebu and arm-soc trees.

drivers/cpuidle/Kconfig.arm line 5

Quoted 5 lines

drivers/cpuidle/Kconfig.arm +2 -2

-config ARM_ARMADA_370_XP_CPUIDLE
-	bool "CPU Idle Driver for Armada 370/XP family processors"
+config ARM_MVEBU_V7_CPUIDLE
+	bool "CPU Idle Driver for mvebu v7 family processors"

Actually, what worries me a bit of that Dove is a ARMv7 processor of the mvebu family, but which is not using this cpuidle driver. Do you expect Dove to be able to use this driver in the future?

drivers/cpuidle/Kconfig.arm line 8

Quoted 5 lines

drivers/cpuidle/Kconfig.arm +1 -1

 	depends on ARCH_MVEBU
 	help
-	  Select this to enable cpuidle on Armada 370/XP processors.
+	  Select this to enable cpuidle on Armada 370, 385 and XP processors.

385 -> 38x, because 380 and 385 are both capable of using this driver.

drivers/cpuidle/cpuidle-armada-370-xp.c

Quoted 6 lines

drivers/cpuidle/cpuidle-armada-370-xp.c deleted +0 -0

deleted file mode 100644
index a5fba0287bfb..000000000000

You haven't enabled rename detection in git? Or git isn't detecting the rename because of the number of changes in the file?

In order to prepare the add of new SoCs supports for this cpuidle

driver, this patch moves the description of the state in the platform part. Indeed the number of the cpuidle state, and the value of the flag used will vary depending of the SoC.

Quoted 1 line

In order to prepare the add of new SoCs supports for this cpuidle

"the add of new SoCs supports for this cpuidle" -> "the addition of the support for more SoCs in this cpuidle".

Quoted 3 lines

driver, this patch moves the description of the state in the platform part. Indeed the number of the cpuidle state, and the value of the flag used will vary depending of the SoC.

I actually don't really agree with the reasoning here. We can perfectly fine have several separate compatible strings, one for each SoC. We do this for pinctrl, for clocks, and for many other drivers. Why should it be different for the cpuidle driver? I know the function to enter idle will most likely implemented in arch/arm/mach-mvebu/ because those functions are generally too tightly coupled with a bunch of system registers changes, but the description of the different idle states can just as well be inside the cpuidle driver itself.

Unlike the Armada XP the Armada 370 always power done the L2 cache, so

it have only 2 cpuidle states. Thanks to the previous patches, adding

the support for this new SoCs required only the modification of the

architecture specific part.

The message in case of failure to suspend the system was switched from warn to debug. Indeed due to the "slow exit process from the deep idle state" in Armada 370, this situation happens quite often. Using the _debug version avoids spamming the kernel logs, but still allows to enable it if needed.

arch/arm/mach-mvebu/pmsu.c +7 -0

+static struct mvebu_v7_cpuidle armada_370_cpuidle = {
+	.mvebu_v7_idle_driver = {
+		.name			= "armada_370_idle",
+		.states[0]		= ARM_CPUIDLE_WFI_STATE,
+		.states[1]		= {
+			.exit_latency		= 100,
+			.power_usage		= 5,

arch/arm/mach-mvebu/pmsu.c +29 -1

+
 static struct mvebu_v7_cpuidle armada_xp_cpuidle = {
 	.mvebu_v7_idle_driver = {
 		.name			= "armada_xp_idle",

@@ -328,6 +346,31 @@ static struct mvebu_v7_cpuidle armada_xp_cpuidle = {
 	.mvebu_v7_cpu_suspend = armada_xp_370_cpu_suspend,
 };
 
+static __init bool armada_370_cpuidle_init(void)
+{
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, "marvell,coherency-fabric");
+	if (!np)
+		return false;
+	of_node_put(np);
+
+	/*
+	 * On Armada 370, there is "a slow exit process from the deep
+	 * idle state due to heavy L1/L2 cache cleanup operations
+	 * performed by the BootROM software". To avoid this, we
+	 * replace the restart code of the bootrom by a a simple jump
+	 * to the boot address. Then the code located at this boot
+	 * address will take care of the initialization.
+	 */
+	mvebu_boot_addr_wa(ARMADA_370_CRYPT0_ENG_ID, pmsu_mp_phys_base +
+			PMSU_BOOT_ADDR_REDIRECT_OFFSET(0));
+
+	mvebu_cpu_resume = armada_370_xp_cpu_resume;
+	mvebu_v7_cpuidle_device.dev.platform_data = &armada_370_cpuidle;
+	return true;
+}
+
 static __init bool armada_xp_cpuidle_init(void)
 {
 	struct device_node *np;

@@ -345,6 +388,9 @@ static struct of_device_id of_cpuidle_table[] __initdata = {
 	{ .compatible = "marvell,armadaxp",
 	  .data = (void *)armada_xp_cpuidle_init,
 	},
+	{ .compatible = "marvell,armada370",
+	  .data = (void *)armada_370_cpuidle_init,
+	},
 	{ /* end of list */ },
 };
 

@@ -373,18 +419,6 @@ static int __init mvebu_v7_cpu_pm_init(void)
 		return 0;
 	of_node_put(np);
 
-	/*

Quoted 1 line

it have only 2 cpuidle states. Thanks to the previous patches, adding

have -> has.

Quoted 1 line

the support for this new SoCs required only the modification of the

required -> requires.

Quoted 1 line

architecture specific part.

May not be true if we move the cpuidle states back into the cpuidle driver, as I suggested in a comment to one of the previous patch.

Quoted 6 lines

The message in case of failure to suspend the system was switched from warn to debug. Indeed due to the "slow exit process from the deep idle state" in Armada 370, this situation happens quite often. Using the _debug version avoids spamming the kernel logs, but still allows to enable it if needed.

Is it really expected that the system fails to enter the idle state? I must say I don't really see the relation with the slow exit from deep idle state, because the do_armada_xp_370_cpu_suspend() function is only here to *enter* the idle state, so how is entering the deep idle state related to an issue when exiting the idle state?

Quoted 9 lines

arch/arm/mach-mvebu/pmsu.c +0 -9

-	/*
-	 * On Armada 370, there is "a slow exit process from the deep
-	 * idle state due to heavy L1/L2 cache cleanup operations
-	 * performed by the BootROM software". To avoid this, we
-	 * replace the restart code of the bootrom by a a simple jump
-	 * to the boot address. Then the code located at this boot
-	 * address will take care of the initialization.
-	 */
-	if (of_machine_is_compatible("marvell,armada370"))

arch/arm/mach-mvebu/pmsu.c static int __init mvebu_v7_cpu_pm_init(void) · line 386

Quoted 2 lines

arch/arm/mach-mvebu/pmsu.c +0 -2

-		mvebu_boot_addr_wa(ARMADA_370_CRYPT0_ENG_ID, pmsu_mp_phys_base +
-				PMSU_BOOT_ADDR_REDIRECT_OFFSET(0));

So we're almost entirely reverting/moving the code added by "[PATCH 05/16] ARM: mvebu: Add workaround for cpuidle support for Armada 370". Maybe there is an issue in your series, and the workaround for cpuidle support on Armada 370 should only be added in this patch, and not earlier?

Unlike the Armada XP and the Armada 370, this SoC uses a Cortex A9 core.

Beside this, the main difference for the cpu idle is the way to handle the L2 cache and the use of SCU.

+static void __iomem *scu_base;

arch/arm/mach-mvebu/pmsu.c +2 -0

  out:
 	of_node_put(np);
 	return ret;

@@ -260,6 +277,27 @@ static int armada_xp_370_cpu_suspend(unsigned long deepidle)
 	return cpu_suspend(deepidle, do_armada_xp_370_cpu_suspend);
 }
 
+static noinline int do_armada_38x_cpu_suspend(unsigned long deepidle)
+{

arch/arm/mach-mvebu/pmsu.c +7 -0

+}
+
+static int armada_38x_cpu_suspend(unsigned long deepidle)
+{
+	return cpu_suspend(false, do_armada_38x_cpu_suspend);
+}
+
 /* No locking is needed because we only access per-CPU registers */
 static noinline void mvebu_v7_pmsu_idle_restore(void)
 {

arch/arm/mach-mvebu/pmsu.c +6 -0

 	/* 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;

@@ -320,6 +357,23 @@ static struct mvebu_v7_cpuidle armada_370_cpuidle = {
 	.mvebu_v7_cpu_suspend = armada_xp_370_cpu_suspend,
 };
 
+static struct mvebu_v7_cpuidle armada_38x_cpuidle = {
+	.mvebu_v7_idle_driver = {
+		.name			= "armada_38x_idle",
+		.states[0]		= ARM_CPUIDLE_WFI_STATE,
+		.states[1]		= {
+			.exit_latency		= 10,

arch/arm/mach-mvebu/pmsu.c +10 -0

+
+	/* Set up reset mask when powering down the cpus */
+	reg = readl(mpsoc_base + MPCORE_RESET_CTL);
+	reg |= MPCORE_RESET_CTL_L2;
+	reg |= MPCORE_RESET_CTL_DEBUG;
+	writel(reg, mpsoc_base + MPCORE_RESET_CTL);
+	iounmap(mpsoc_base);
+	of_node_put(np);
+
+	/* Set up delay */

arch/arm/mach-mvebu/pmsu.c +0 -0

 	{ /* end of list */ },
 };
 

arch/arm/mach-mvebu/pmsu_ll.S +0 -0

index 3b702a16bd3d..15b823dff61a 100644

arch/arm/mach-mvebu/pmsu_ll.S +4 -0

+	bl	v7_invalidate_l1
+	mrc     p15, 4, r1, c15, c0	@ get SCU base address
+	orr	r1, r1, #0x8		@ SCU CPU Power Status Register
+	mrc	15, 0, r0, cr0, cr0, 5	@ get the CPU ID

Quoted 2 lines

Unlike the Armada XP and the Armada 370, this SoC uses a Cortex A9 core.

Isn't there some more details missing in this sentence? When I read it, at the end of the sentence, I'm wondering "and so what? what does this implies in terms of cpuidle support?".

Quoted 3 lines

Beside this, the main difference for the cpu idle is the way to handle the L2 cache and the use of SCU.

arch/arm/mach-mvebu/pmsu.c static int __init mvebu_v7_pmsu_init(void) · line 171

Quoted 3 lines

arch/arm/mach-mvebu/pmsu.c +1 -0

 		pr_err("unable to request region\n");
 		ret = -EBUSY;
+

This change.

Quoted 3 lines

(quoted diff) +0 -0

 		goto out;
 	}

arch/arm/mach-mvebu/pmsu.c static int __init mvebu_v7_pmsu_init(void) · line 166

Quoted 5 lines

arch/arm/mach-mvebu/pmsu.c +0 -1

@@ -163,7 +181,6 @@ static int __init mvebu_v7_pmsu_init(void)
 		ret = -ENOMEM;
 		goto out;
 	}
-

And this one look like spurious changes not related to the patch.

Quoted 8 lines

arch/arm/mach-mvebu/pmsu.c +0 -0

  out:
 	of_node_put(np);
 	return ret;

@@ -260,6 +277,27 @@ static int armada_xp_370_cpu_suspend(unsigned long deepidle)
 	return cpu_suspend(deepidle, do_armada_xp_370_cpu_suspend);
 }
 

arch/arm/mach-mvebu/pmsu.c static int armada_xp_370_cpu_suspend(unsigned long deepidle) · line 291

Quoted 12 lines

arch/arm/mach-mvebu/pmsu.c +12 -0

+static noinline int do_armada_38x_cpu_suspend(unsigned long deepidle)
+{
+	mvebu_v7_pmsu_idle_prepare(deepidle, false);
+	/*
+	 * Already flushed cache, but do it again as the outer cache
+	 * functions dirty the cache with spinlocks
+	 */
+	v7_exit_coherency_flush(louis);
+
+	scu_power_mode(scu_base, SCU_PM_POWEROFF);
+
+	cpu_do_idle();

I see cpu_do_idle() does dsb() and wfi(), so why don't we use in the do_armada_370_xp_cpu_suspend() function ?

Review static int armada_xp_370_cpu_suspend(unsigned long deepidle) · line 293

Quoted 3 lines

(quoted diff) +2 -0

+
+	return 1;

You return 1 here, but in the do_armada_370_xp_cpu_suspend() function you return zero. Is the return value being used? Why use 0 in one case and 1 in the other?

arch/arm/mach-mvebu/pmsu.c static __init bool armada_370_cpuidle_init(void) · line 437

Quoted 9 lines

arch/arm/mach-mvebu/pmsu.c +9 -0

+{
+	struct device_node *np;
+	void __iomem *mpsoc_base;
+	u32 reg;
+
+	np = of_find_compatible_node(NULL, NULL,
+				"marvell,armada-380-coherency-fabric");
+	if (!np)
+		return false;

		return -ENODEV;

arch/arm/mach-mvebu/pmsu.c static __init bool armada_370_cpuidle_init(void) · line 443

Quoted 6 lines

arch/arm/mach-mvebu/pmsu.c +6 -0

+	of_node_put(np);
+
+	np = of_find_compatible_node(NULL, NULL,
+				"marvell,armada-380-mpcore-soc-ctrl");
+	if (!np)
+		return false;

		return -ENODEV;

arch/arm/mach-mvebu/pmsu.c static __init bool armada_370_cpuidle_init(void) · line 445

Quoted 2 lines

arch/arm/mach-mvebu/pmsu.c +2 -0

+	mpsoc_base = of_iomap(np, 0);
+	WARN_ON(!mpsoc_base);

WARN_ON() seems a bit weak for something that will make the next line crash the kernel. What about:

	if (!mpsoc_base)
		return -ENOMEM;

I think the of_node_put(np) should be here.

arch/arm/mach-mvebu/pmsu_ll.S ARM_BE8(setend be ) @ go BE8 if entered LE · line 28

Quoted 4 lines

arch/arm/mach-mvebu/pmsu_ll.S +3 -0

+ENTRY(armada_38x_cpu_resume)
+	/* do we need it for Armada 38x*/
+ARM_BE8(setend	be )			@ go BE8 if entered LE

We logically needed this, but I've confirmed, and indeed it is really needed for cpuidle to work properly on Armada 38x in big-endian configuration.

This patch set adds the CPU Idle support for two mvebu SoCs: the Armada 370 and the Armada 38x. As there are common parts with the current support of cpuidle of the Armada XP. I made the code even more generic. Thanks to this change, adding a new SoC is now only done in the architecture specific part and we won't have to touch the cpuidle driver anymore for this.

Gregory,

Quoted 6 lines · Gregory CLEMENT

This patch set adds the CPU Idle support for two mvebu SoCs: the Armada 370 and the Armada 38x. As there are common parts with the current support of cpuidle of the Armada XP. I made the code even more generic. Thanks to this change, adding a new SoC is now only done in the architecture specific part and we won't have to touch the cpuidle driver anymore for this.

I'd like to do some testing of this patch series, especially in big endian situations. Do you have a Git repository where this version was pushed?

Thanks,

Thomas

Quoted 3 lines

I'd like to do some testing of this patch series, especially in big endian situations. Do you have a Git repository where this version was pushed?

Yes sure, the branch CPU-Idle-mvebu-next-3.16-rc2-V1 on
https://github.com/MISL-EBU-System-SW/mainline-public.git