Re: [PATCH 1/2] ARM: mvebu: Disable CPU Hotplug if PMSU is not available

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Gregory CLEMENT,

On Sat, 31 Jan 2015 14:57:47 +0100, Gregory CLEMENT wrote:
> The CPU hotplug needs accessing the PMSU register through the CPU Idle
> functions. If we fail accessing these registers we need disabling

we need disabling -> we must disable

> the CPU hotplug.

the CPU hotplug support.

> 
> Cc: <stable@xxxxxxxxxxxxxxx> # v3.17+
> Signed-off-by: Gregory CLEMENT <gregory.clement@xxxxxxxxxxxxxxxxxx>
> ---
>  arch/arm/mach-mvebu/pmsu.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
> index d8ab605a44fa..0a8e2f548466 100644
> --- a/arch/arm/mach-mvebu/pmsu.c
> +++ b/arch/arm/mach-mvebu/pmsu.c
> @@ -159,6 +159,11 @@ int mvebu_setup_boot_addr_wa(unsigned int crypto_eng_target,
>  	return 0;
>  }
>  
> +/*
> + * The CPU hotplug

support

> needs to access the PMSU register 

registers

> through the CPU
> + * Idle functions,

I don't think "through" is appropriate here. Maybe something like: "The
CPU hotplug support relies on putting the CPU in a deep sleep state,
which requires access to PMSU registers. If the PMSU is not available,
we cannot provide the CPU hotplug functionality."

Are you sure that the cpu_hotplug_disable() function is meant to be
accessed from platform code? It is only used internally in
kernel/cpu.c, and called from kernel/reboot.c.

I rather believe that it's the ->cpu_kill() function from
smp_operations that should return an error code when you cannot do CPU
hotplug. However, on Armada 38x, it doesn't make sense, because you
*must* have the PMSU available to boot secondary CPUs (the secondary
CPU boot address is configured in a PMSU register). So it is not
possible to have secondary CPUs to turn off using CPU hotplug if the
PMSU is not available.

It would be possible on Armada 375, but we don't support CPU hotplug on
Armada 375, and it anyway wouldn't need the PMSU since it doesn't have
a PMSU.

And on Armada XP, it's the same as Armada 38x: the PMSU must be
available to boot secondary CPUs.

So in the end, I don't understand what this patch is fixing.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]