2014-10-23 18:14:58

by Gregory CLEMENT

[permalink] [raw]
Subject: [PATCH 0/4] Hot plug support for Armada 38x

Hi,

This patch set adds the hot plug and also kexec support for the Armada
38x Socs.

The first patch was done in order to have the same code between Armada
XP and the Cortex A9 based mvebu SoCs. In order to ensure the the
backward compatibility for the device tree, it is only a preliminary
work for it.

The second patch moves the SCU power up sequence in a dedicated
assembly function. It was done in order to reuse it in the 3rd patch.

The third patch fixes the secondary startup for the cortex A9 mvebu
SoC. Indeed, the initial code was written with the assumption the SCU
will be always power on, which is not only true especially in the
kexec case.

These 2 patches may worth to be pushed to the stable kernel.

Then the last patch adds the CPU hotplug support for Armada 38x. I
tested the hotplug using the /sys/devices/system/cpu/cpu1/online
virtual file. I also tested the kexec feature and managed to switch
to a new kernel using kexec.

Thanks,

Gregory


Gregory CLEMENT (4):
ARM: mvebu: Clean-up the Armada XP support
ARM: mvebu: Move SCU power up in a function
ARM: mvebu: Fix secondary startup for Cortex A9 SoC
ARM: mvebu: Implement CPU hotplug support for Armada 38x

arch/arm/mach-mvebu/armada-370-xp.h | 6 -----
arch/arm/mach-mvebu/board-v7.c | 4 +++
arch/arm/mach-mvebu/coherency.c | 1 -
arch/arm/mach-mvebu/cpu-reset.c | 1 -
arch/arm/mach-mvebu/headsmp-a9.S | 1 +
arch/arm/mach-mvebu/platsmp-a9.c | 53 +++++++++++++++++++++++++++++++++++--
arch/arm/mach-mvebu/platsmp.c | 2 ++
arch/arm/mach-mvebu/pmsu.c | 3 +--
arch/arm/mach-mvebu/pmsu.h | 2 ++
arch/arm/mach-mvebu/pmsu_ll.S | 20 +++++++++-----
10 files changed, 74 insertions(+), 19 deletions(-)

--
1.9.1


2014-10-23 18:15:14

by Gregory CLEMENT

[permalink] [raw]
Subject: [PATCH 2/4] ARM: mvebu: Move SCU power up in a function

This will allow reusing the same function in the secondary_startup
for the Cortex A9 SoC.

Signed-off-by: Gregory CLEMENT <[email protected]>
---
arch/arm/mach-mvebu/pmsu_ll.S | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-mvebu/pmsu_ll.S b/arch/arm/mach-mvebu/pmsu_ll.S
index a945756cfb45..0505010474a9 100644
--- a/arch/arm/mach-mvebu/pmsu_ll.S
+++ b/arch/arm/mach-mvebu/pmsu_ll.S
@@ -12,6 +12,18 @@
#include <linux/linkage.h>
#include <asm/assembler.h>

+
+ENTRY(power_up_scu)
+ 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
+ and r0, r0, #15
+ add r1, r1, r0
+ mov r0, #0x0
+ strb r0, [r1] @ switch SCU power state to Normal mode
+ ret lr
+ENDPROC(power_up_scu)
+
/*
* This is the entry point through which CPUs exiting cpuidle deep
* idle state are going.
@@ -27,13 +39,7 @@ ENTRY(armada_38x_cpu_resume)
/* do we need it for Armada 38x*/
ARM_BE8(setend be ) @ go BE8 if entered LE
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
- and r0, r0, #15
- add r1, r1, r0
- mov r0, #0x0
- strb r0, [r1] @ switch SCU power state to Normal mode
+ bl power_up_scu
b cpu_resume
ENDPROC(armada_38x_cpu_resume)

--
1.9.1

2014-10-23 18:15:12

by Gregory CLEMENT

[permalink] [raw]
Subject: [PATCH 3/4] ARM: mvebu: Fix secondary startup for Cortex A9 SoC

During the secondary startup the SCU was assumed to be in normal
mode. It is not always the case, and especially after a kexec. This
commit adds the needed sequence to put the SCU in normal mode.

Signed-off-by: Gregory CLEMENT <[email protected]>
---
arch/arm/mach-mvebu/headsmp-a9.S | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-mvebu/headsmp-a9.S b/arch/arm/mach-mvebu/headsmp-a9.S
index be51c998c0cd..2a756ef5c4ba 100644
--- a/arch/arm/mach-mvebu/headsmp-a9.S
+++ b/arch/arm/mach-mvebu/headsmp-a9.S
@@ -22,5 +22,6 @@
ENTRY(mvebu_cortex_a9_secondary_startup)
ARM_BE8(setend be)
bl v7_invalidate_l1
+ bl power_up_scu
b secondary_startup
ENDPROC(mvebu_cortex_a9_secondary_startup)
--
1.9.1

2014-10-23 18:15:10

by Gregory CLEMENT

[permalink] [raw]
Subject: [PATCH 1/4] ARM: mvebu: Clean-up the Armada XP support

This patch removes the unneeded include of the armada-370-xp.h header.

It also moves some declarations from this file into more accurate
places.

Finally, it also adds a comment explaining that we can't remove yet the
smp field in the dt machine struct due to backward compatibly of the
device tree.

In a few releases, when the old device tree will be obsolete, we will be
able to remove the smp field and then the armada-370-xp.h header.

Signed-off-by: Gregory CLEMENT <[email protected]>
---
arch/arm/mach-mvebu/armada-370-xp.h | 6 ------
arch/arm/mach-mvebu/board-v7.c | 4 ++++
arch/arm/mach-mvebu/coherency.c | 1 -
arch/arm/mach-mvebu/cpu-reset.c | 1 -
arch/arm/mach-mvebu/platsmp.c | 2 ++
arch/arm/mach-mvebu/pmsu.c | 1 -
arch/arm/mach-mvebu/pmsu.h | 1 +
7 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-mvebu/armada-370-xp.h b/arch/arm/mach-mvebu/armada-370-xp.h
index 84cd90d9b860..c55bbf81de0e 100644
--- a/arch/arm/mach-mvebu/armada-370-xp.h
+++ b/arch/arm/mach-mvebu/armada-370-xp.h
@@ -16,14 +16,8 @@
#define __MACH_ARMADA_370_XP_H

#ifdef CONFIG_SMP
-#include <linux/cpumask.h>
-
-#define ARMADA_XP_MAX_CPUS 4
-
void armada_xp_secondary_startup(void);
extern struct smp_operations armada_xp_smp_ops;
#endif

-int armada_370_xp_pmsu_idle_enter(unsigned long deepidle);
-
#endif /* __MACH_ARMADA_370_XP_H */
diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
index 6478626e3ff6..e51bb1ddffbf 100644
--- a/arch/arm/mach-mvebu/board-v7.c
+++ b/arch/arm/mach-mvebu/board-v7.c
@@ -206,6 +206,10 @@ static const char * const armada_370_xp_dt_compat[] = {
DT_MACHINE_START(ARMADA_370_XP_DT, "Marvell Armada 370/XP (Device Tree)")
.l2c_aux_val = 0,
.l2c_aux_mask = ~0,
+/*
+ * The following field (.smp) is stil needed to ensure backward
+ * compatibility with the old device trees
+ */
.smp = smp_ops(armada_xp_smp_ops),
.init_machine = mvebu_dt_init,
.init_irq = mvebu_init_irq,
diff --git a/arch/arm/mach-mvebu/coherency.c b/arch/arm/mach-mvebu/coherency.c
index 2bdc3233abe2..d1b0a173e4a6 100644
--- a/arch/arm/mach-mvebu/coherency.c
+++ b/arch/arm/mach-mvebu/coherency.c
@@ -33,7 +33,6 @@
#include <asm/smp_plat.h>
#include <asm/cacheflush.h>
#include <asm/mach/map.h>
-#include "armada-370-xp.h"
#include "coherency.h"
#include "mvebu-soc-id.h"

diff --git a/arch/arm/mach-mvebu/cpu-reset.c b/arch/arm/mach-mvebu/cpu-reset.c
index 60fb53787004..4a2cadd6b48e 100644
--- a/arch/arm/mach-mvebu/cpu-reset.c
+++ b/arch/arm/mach-mvebu/cpu-reset.c
@@ -15,7 +15,6 @@
#include <linux/of_address.h>
#include <linux/io.h>
#include <linux/resource.h>
-#include "armada-370-xp.h"

static void __iomem *cpu_reset_base;
static size_t cpu_reset_size;
diff --git a/arch/arm/mach-mvebu/platsmp.c b/arch/arm/mach-mvebu/platsmp.c
index 895dc373c8a1..622315c185b2 100644
--- a/arch/arm/mach-mvebu/platsmp.c
+++ b/arch/arm/mach-mvebu/platsmp.c
@@ -30,6 +30,8 @@
#include "pmsu.h"
#include "coherency.h"

+#define ARMADA_XP_MAX_CPUS 4
+
#define AXP_BOOTROM_BASE 0xfff00000
#define AXP_BOOTROM_SIZE 0x100000

diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
index bbd8664d1bac..5af58927a56d 100644
--- a/arch/arm/mach-mvebu/pmsu.c
+++ b/arch/arm/mach-mvebu/pmsu.c
@@ -39,7 +39,6 @@
#include <asm/suspend.h>
#include <asm/tlbflush.h>
#include "common.h"
-#include "armada-370-xp.h"


#define PMSU_BASE_OFFSET 0x100
diff --git a/arch/arm/mach-mvebu/pmsu.h b/arch/arm/mach-mvebu/pmsu.h
index 6b58c1fe2b0d..e1eb4959a2d4 100644
--- a/arch/arm/mach-mvebu/pmsu.h
+++ b/arch/arm/mach-mvebu/pmsu.h
@@ -18,4 +18,5 @@ int mvebu_setup_boot_addr_wa(unsigned int crypto_eng_target,

void mvebu_v7_pmsu_idle_exit(void);

+int armada_370_xp_pmsu_idle_enter(unsigned long deepidle);
#endif /* __MACH_370_XP_PMSU_H */
--
1.9.1

2014-10-23 18:16:09

by Gregory CLEMENT

[permalink] [raw]
Subject: [PATCH 4/4] ARM: mvebu: Implement CPU hotplug support for Armada 38x

This commit implements CPU hotplug support for the Marvell Armada 38x
platform. Similarly to what was done for the Armada XP, this commit:

* Implements the ->cpu_die() function of SMP operations by calling
armada_38x_do_cpu_suspend() to enter the deep idle state for
CPUs going offline.

* Implements a dummy ->cpu_kill() function, simply needed for the
kernel to know we have CPU hotplug support.

* The mvebu_cortex_a9_boot_secondary() function makes sure to wake up
the CPU if waiting in deep idle state by sending an IPI before
deasserting the CPUs from reset. This is because
mvebu_cortex_a9_boot_secondary() is now used in two different
situations: for the initial boot of secondary CPUs (where CPU reset
deassert is used to wake up CPUs) and for CPU hotplug (where an IPI
is used to take CPU out of deep idle).

* At boot time, we exit from the idle state in the
->smp_secondary_init() hook.

This commit has been tested using CPU hotplug through sysfs
(/sys/devices/system/cpu/cpuX/online) and using kexec.

Signed-off-by: Gregory CLEMENT <[email protected]>
---
arch/arm/mach-mvebu/platsmp-a9.c | 53 ++++++++++++++++++++++++++++++++++++++--
arch/arm/mach-mvebu/pmsu.c | 2 +-
arch/arm/mach-mvebu/pmsu.h | 1 +
3 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-mvebu/platsmp-a9.c b/arch/arm/mach-mvebu/platsmp-a9.c
index 47a71a924b96..2ec1a42b4321 100644
--- a/arch/arm/mach-mvebu/platsmp-a9.c
+++ b/arch/arm/mach-mvebu/platsmp-a9.c
@@ -43,21 +43,70 @@ static int __cpuinit mvebu_cortex_a9_boot_secondary(unsigned int cpu,
else
mvebu_pmsu_set_cpu_boot_addr(hw_cpu, mvebu_cortex_a9_secondary_startup);
smp_wmb();
+
+ /*
+ * Doing this before deasserting the CPUs is needed to wake up CPUs
+ * in the offline state after using CPU hotplug.
+ */
+ arch_send_wakeup_ipi_mask(cpumask_of(cpu));
+
ret = mvebu_cpu_reset_deassert(hw_cpu);
if (ret) {
pr_err("Could not start the secondary CPU: %d\n", ret);
return ret;
}
- arch_send_wakeup_ipi_mask(cpumask_of(cpu));

return 0;
}
+/*
+ * When a CPU is brought back online, either through CPU hotplug, or
+ * because of the boot of a kexec'ed kernel, the PMSU configuration
+ * for this CPU might be in the deep idle state, preventing this CPU
+ * from receiving interrupts. Here, we therefore take out the current
+ * CPU from this state, which was entered by armada_38x_cpu_die()
+ * below.
+ */
+static void armada_38x_secondary_init(unsigned int cpu)
+{
+ mvebu_v7_pmsu_idle_exit();
+}
+
+#ifdef CONFIG_HOTPLUG_CPU
+static void armada_38x_cpu_die(unsigned int cpu)
+{
+ /*
+ * CPU hotplug is implemented by putting offline CPUs into the
+ * deep idle sleep state.
+ */
+ armada_38x_do_cpu_suspend(true);
+}
+
+/*
+ * We need a dummy function, so that platform_can_cpu_hotplug() knows
+ * we support CPU hotplug. However, the function does not need to do
+ * anything, because CPUs going offline can enter the deep idle state
+ * by themselves, without any help from a still alive CPU.
+ */
+static int armada_38x_cpu_kill(unsigned int cpu)
+{
+ return 1;
+}
+#endif

static struct smp_operations mvebu_cortex_a9_smp_ops __initdata = {
.smp_boot_secondary = mvebu_cortex_a9_boot_secondary,
};

+static struct smp_operations armada_38x_smp_ops __initdata = {
+ .smp_boot_secondary = mvebu_cortex_a9_boot_secondary,
+ .smp_secondary_init = armada_38x_secondary_init,
+#ifdef CONFIG_HOTPLUG_CPU
+ .cpu_die = armada_38x_cpu_die,
+ .cpu_kill = armada_38x_cpu_kill,
+#endif
+};
+
CPU_METHOD_OF_DECLARE(mvebu_armada_375_smp, "marvell,armada-375-smp",
&mvebu_cortex_a9_smp_ops);
CPU_METHOD_OF_DECLARE(mvebu_armada_380_smp, "marvell,armada-380-smp",
- &mvebu_cortex_a9_smp_ops);
+ &armada_38x_smp_ops);
diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
index 5af58927a56d..5a757f929927 100644
--- a/arch/arm/mach-mvebu/pmsu.c
+++ b/arch/arm/mach-mvebu/pmsu.c
@@ -311,7 +311,7 @@ static int armada_370_xp_cpu_suspend(unsigned long deepidle)
return cpu_suspend(deepidle, armada_370_xp_pmsu_idle_enter);
}

-static int armada_38x_do_cpu_suspend(unsigned long deepidle)
+int armada_38x_do_cpu_suspend(unsigned long deepidle)
{
unsigned long flags = 0;

diff --git a/arch/arm/mach-mvebu/pmsu.h b/arch/arm/mach-mvebu/pmsu.h
index e1eb4959a2d4..c2c95db4f648 100644
--- a/arch/arm/mach-mvebu/pmsu.h
+++ b/arch/arm/mach-mvebu/pmsu.h
@@ -19,4 +19,5 @@ int mvebu_setup_boot_addr_wa(unsigned int crypto_eng_target,
void mvebu_v7_pmsu_idle_exit(void);

int armada_370_xp_pmsu_idle_enter(unsigned long deepidle);
+int armada_38x_do_cpu_suspend(unsigned long deepidle);
#endif /* __MACH_370_XP_PMSU_H */
--
1.9.1

2014-10-23 18:44:32

by Thomas Petazzoni

[permalink] [raw]
Subject: Re: [PATCH 1/4] ARM: mvebu: Clean-up the Armada XP support

Dear Gregory CLEMENT,

On Thu, 23 Oct 2014 20:14:27 +0200, Gregory CLEMENT wrote:

> +/*
> + * The following field (.smp) is stil needed to ensure backward

stil -> still

> + * compatibility with the old device trees

It would be good to be more specific here:

The following field (.smp) is still needed to ensure backward
compatibility with old Device Trees that were not specifying
the cpus enable-method property.

Other than that, looks good to me.

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

2014-10-24 12:05:55

by Thomas Petazzoni

[permalink] [raw]
Subject: Re: [PATCH 2/4] ARM: mvebu: Move SCU power up in a function

Dear Gregory CLEMENT,

On Thu, 23 Oct 2014 20:14:28 +0200, Gregory CLEMENT wrote:

> +ENTRY(power_up_scu)
> + 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
> + and r0, r0, #15
> + add r1, r1, r0
> + mov r0, #0x0
> + strb r0, [r1] @ switch SCU power state to Normal mode
> + ret lr
> +ENDPROC(power_up_scu)

Since this function is not static, I think it might be a good idea to
use a prefix that makes it more specific to the platform in order to
not pollute the global namespace. Maybe something like
'armada_38x_scu_power_up', or something like that.

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

2014-10-24 12:11:26

by Gregory CLEMENT

[permalink] [raw]
Subject: Re: [PATCH 2/4] ARM: mvebu: Move SCU power up in a function

Hi Thomas,

On 24/10/2014 14:05, Thomas Petazzoni wrote:
> Dear Gregory CLEMENT,
>
> On Thu, 23 Oct 2014 20:14:28 +0200, Gregory CLEMENT wrote:
>
>> +ENTRY(power_up_scu)
>> + 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
>> + and r0, r0, #15
>> + add r1, r1, r0
>> + mov r0, #0x0
>> + strb r0, [r1] @ switch SCU power state to Normal mode
>> + ret lr
>> +ENDPROC(power_up_scu)
>
> Since this function is not static, I think it might be a good idea to
> use a prefix that makes it more specific to the platform in order to
> not pollute the global namespace. Maybe something like
> 'armada_38x_scu_power_up', or something like that.

Yes given the fact that this function is declared in the mach-mveu folder
it is sensible to use a more specific name. However the function itself
is not specific at all to an SoC. This function could be used for any Cortex A9
using the SCU.


Thanks,

Gregory



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

2014-10-24 12:20:04

by Thomas Petazzoni

[permalink] [raw]
Subject: Re: [PATCH 2/4] ARM: mvebu: Move SCU power up in a function

Dear Gregory CLEMENT,

On Fri, 24 Oct 2014 14:11:17 +0200, Gregory CLEMENT wrote:

> > Since this function is not static, I think it might be a good idea to
> > use a prefix that makes it more specific to the platform in order to
> > not pollute the global namespace. Maybe something like
> > 'armada_38x_scu_power_up', or something like that.
>
> Yes given the fact that this function is declared in the mach-mveu folder
> it is sensible to use a more specific name. However the function itself
> is not specific at all to an SoC. This function could be used for any Cortex A9
> using the SCU.

Indeed, but as you say, it's right now defined in the mvebu specific
code, so we shouldn't pollute the global namespace.

Note that there is already a scu_power_mode() that does the same thing,
but implemented in C, so I'm not sure you can call it at search an early
point.

Best regards,

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

2014-10-24 12:28:46

by Gregory CLEMENT

[permalink] [raw]
Subject: Re: [PATCH 2/4] ARM: mvebu: Move SCU power up in a function

Hi Thomas,

On 24/10/2014 14:19, Thomas Petazzoni wrote:
> Dear Gregory CLEMENT,
>
> On Fri, 24 Oct 2014 14:11:17 +0200, Gregory CLEMENT wrote:
>
>>> Since this function is not static, I think it might be a good idea to
>>> use a prefix that makes it more specific to the platform in order to
>>> not pollute the global namespace. Maybe something like
>>> 'armada_38x_scu_power_up', or something like that.
>>
>> Yes given the fact that this function is declared in the mach-mveu folder
>> it is sensible to use a more specific name. However the function itself
>> is not specific at all to an SoC. This function could be used for any Cortex A9
>> using the SCU.
>
> Indeed, but as you say, it's right now defined in the mvebu specific
> code, so we shouldn't pollute the global namespace.
>
> Note that there is already a scu_power_mode() that does the same thing,

I know this function: I used it for the idle support.

> but implemented in C, so I'm not sure you can call it at search an early
> point.

you meant "such" instead of "search" I guess, and yes unfortunately we can't use
a C function at this step.

Gregory



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