Re: [PATCH 1/1] ARM: STi: Restore secondary CPU's bringup

[Patrice CHOTARD]

Hi Russell

On 12/18/18 4:52 PM, Russell King - ARM Linux wrote:
> On Tue, Dec 18, 2018 at 03:48:13PM +0100, patrice.chotard@xxxxxx wrote:
>> From: Patrice Chotard <patrice.chotard@xxxxxx>
>>
>> Due to pen_release and boot_lock removal, secondary CPU's bringup
>> was broken. Restore CPU's bringup by reworking properly
>> .smp_prepare_cpus and .smp_boot_secondary STi callbacks.
> 
> Sorry, maybe I don't understand your commit message, but you seem to be
> saying that removal of the pen_release and boot_lock broke STi's secondary
> CPU bring up?  Please clarify, and explain how that happened.

Correct, CPU1 failed to come online.

It seems that writing secondary_startup address at cpu-release-addr in
.smp_prepare_cpus callback was too early.

Doing it in .smp_boot_secondary callback, insures that secondary_data
struct is populated in __cpu_up() (stack, pgdir and swapper_pg_dir fields).

If you are ok, i will pick up your patch [1] and this one to prepare a
STi pull-request.

Thanks

Patrice

[1] https://patchwork.kernel.org/patch/10729479/

> 
> Thanks.
> 
>>
>> Signed-off-by: Patrice Chotard <patrice.chotard@xxxxxx>
>> ---
>>  arch/arm/mach-sti/platsmp.c | 31 ++++++++++++++++---------------
>>  1 file changed, 16 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/arm/mach-sti/platsmp.c b/arch/arm/mach-sti/platsmp.c
>> index 2166850..9bfc93a 100644
>> --- a/arch/arm/mach-sti/platsmp.c
>> +++ b/arch/arm/mach-sti/platsmp.c
>> @@ -28,8 +28,21 @@
>>  
>>  #include "smp.h"
>>  
>> +static u32 __iomem *cpu_strt_ptr;
>> +
>>  static int sti_boot_secondary(unsigned int cpu, struct task_struct *idle)
>>  {
>> +	unsigned long entry_pa = __pa_symbol(secondary_startup);
>> +
>> +	__raw_writel(entry_pa, cpu_strt_ptr);
>> +
>> +	/*
>> +	 * wmb so that data is actually written
>> +	 * before cache flush is done
>> +	 */
>> +	smp_wmb();
>> +	sync_cache_w(cpu_strt_ptr);
>> +
>>  	/*
>>  	 * Send the secondary CPU a soft interrupt, thereby causing
>>  	 * it to jump to the secondary entrypoint.
>> @@ -43,10 +56,8 @@ static void __init sti_smp_prepare_cpus(unsigned int max_cpus)
>>  {
>>  	struct device_node *np;
>>  	void __iomem *scu_base;
>> -	u32 __iomem *cpu_strt_ptr;
>>  	u32 release_phys;
>>  	int cpu;
>> -	unsigned long entry_pa = __pa_symbol(secondary_startup);
>>  
>>  	np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
>>  
>> @@ -74,8 +85,8 @@ static void __init sti_smp_prepare_cpus(unsigned int max_cpus)
>>  		}
>>  
>>  		/*
>> -		 * holding pen is usually configured in SBC DMEM but can also be
>> -		 * in RAM.
>> +		 * cpu-release-addr is usually configured in SBC DMEM but can
>> +		 * also be in RAM.
>>  		 */
>>  
>>  		if (!memblock_is_memory(release_phys))
>> @@ -85,17 +96,7 @@ static void __init sti_smp_prepare_cpus(unsigned int max_cpus)
>>  			cpu_strt_ptr =
>>  				(u32 __iomem *)phys_to_virt(release_phys);
>>  
>> -		__raw_writel(entry_pa, cpu_strt_ptr);
>> -
>> -		/*
>> -		 * wmb so that data is actually written
>> -		 * before cache flush is done
>> -		 */
>> -		smp_wmb();
>> -		sync_cache_w(cpu_strt_ptr);
>> -
>> -		if (!memblock_is_memory(release_phys))
>> -			iounmap(cpu_strt_ptr);
>> +		set_cpu_possible(cpu, true);
>>  	}
>>  }
>>  
>> -- 
>> 1.9.1
>>
>