diff mbox

[04/27] ARM: EXYNOS: Support secondary CPU boot of Exynos4212

Message ID 1397122658-16013-5-git-send-email-cw00.choi@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chanwoo Choi April 10, 2014, 9:37 a.m. UTC
From: Kyungmin Park <kyungmin.park@samsung.com>

This patch fix the offset of CPU boot address and change parameter of smc call
of SMC_CMD_CPU1BOOT command for Exynos4212.

Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-exynos/firmware.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Comments

Olof Johansson April 11, 2014, 1:44 a.m. UTC | #1
On Thu, Apr 10, 2014 at 06:37:15PM +0900, Chanwoo Choi wrote:
> From: Kyungmin Park <kyungmin.park@samsung.com>
> 
> This patch fix the offset of CPU boot address and change parameter of smc call
> of SMC_CMD_CPU1BOOT command for Exynos4212.
> 
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/mach-exynos/firmware.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
> index 932129e..91a911d 100644
> --- a/arch/arm/mach-exynos/firmware.c
> +++ b/arch/arm/mach-exynos/firmware.c
> @@ -18,6 +18,8 @@
>  
>  #include <mach/map.h>
>  
> +#include <plat/cpu.h>
> +
>  #include "smc.h"
>  
>  static int exynos_do_idle(void)
> @@ -28,14 +30,22 @@ static int exynos_do_idle(void)
>  
>  static int exynos_cpu_boot(int cpu)
>  {
> -	exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
> +	if (soc_is_exynos4212())
> +		exynos_smc(SMC_CMD_CPU1BOOT, 0, 0, 0);
> +	else
> +		exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);

	/* <explain why you need this special case on 4212> */
	if (soc_is_exynos4212())
		cpu = 0;

...and then do the call as before.


>  	return 0;
>  }
>  
>  static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>  {
> -	void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
> +	void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c;
>  
> +	if (soc_is_exynos4212())
> +		goto out;
> +	else
> +		boot_reg += 4*cpu;

	if (!soc_is_exynos4212())
		boot_reg += 4 * cpu;

That way you avoid a goto, especially since the "goto out" isn't actually
an "out", it's still doing stuff at the end of the funciton.

> +out:
>  	__raw_writel(boot_addr, boot_reg);
>  	return 0;
>  }
> -- 
> 1.8.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
Chanwoo Choi April 11, 2014, 5:14 a.m. UTC | #2
Hi,

On 04/11/2014 10:44 AM, Olof Johansson wrote:
> On Thu, Apr 10, 2014 at 06:37:15PM +0900, Chanwoo Choi wrote:
>> From: Kyungmin Park <kyungmin.park@samsung.com>
>>
>> This patch fix the offset of CPU boot address and change parameter of smc call
>> of SMC_CMD_CPU1BOOT command for Exynos4212.
>>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  arch/arm/mach-exynos/firmware.c | 14 ++++++++++++--
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
>> index 932129e..91a911d 100644
>> --- a/arch/arm/mach-exynos/firmware.c
>> +++ b/arch/arm/mach-exynos/firmware.c
>> @@ -18,6 +18,8 @@
>>  
>>  #include <mach/map.h>
>>  
>> +#include <plat/cpu.h>
>> +
>>  #include "smc.h"
>>  
>>  static int exynos_do_idle(void)
>> @@ -28,14 +30,22 @@ static int exynos_do_idle(void)
>>  
>>  static int exynos_cpu_boot(int cpu)
>>  {
>> -	exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
>> +	if (soc_is_exynos4212())
>> +		exynos_smc(SMC_CMD_CPU1BOOT, 0, 0, 0);
>> +	else
>> +		exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
> 
> 	/* <explain why you need this special case on 4212> */

It's better to ask system lsi person. We don't know it well.
I got the guide about secondary boot from system lsi.
But, this patch was completely tested.

> 	if (soc_is_exynos4212())
> 		cpu = 0;
> 
> ...and then do the call as before.

OK, I'll modify it as following:

 static int exynos_cpu_boot(int cpu)
 {
+       if (soc_is_exynos4212())
+               cpu = 0;
+
        exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
        return 0;
 }

> 
> 
>>  	return 0;
>>  }
>>  
>>  static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>>  {
>> -	void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
>> +	void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c;
>>  
>> +	if (soc_is_exynos4212())
>> +		goto out;
>> +	else
>> +		boot_reg += 4*cpu;
> 
> 	if (!soc_is_exynos4212())
> 		boot_reg += 4 * cpu;
> 
> That way you avoid a goto, especially since the "goto out" isn't actually
> an "out", it's still doing stuff at the end of the funciton.
> 

OK, I'll remove goto statement and then modify it as your comment.

-       void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
+       void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c;
+
+       if (!soc_is_exynos4212())
+               boot_reg += 4*cpu;

Best Regards,
Chanwoo Choi
Sangbeom Kim April 11, 2014, 6:04 a.m. UTC | #3
Hi,
On 04/11/2014 2:14 PM, Chanwoo Choi wrote:

> >>  {
> >> -	exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
> >> +	if (soc_is_exynos4212())
> >> +		exynos_smc(SMC_CMD_CPU1BOOT, 0, 0, 0);
> >> +	else
> >> +		exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
> >
> > 	/* <explain why you need this special case on 4212> */
> 
> It's better to ask system lsi person. We don't know it well.
> I got the guide about secondary boot from system lsi.
> But, this patch was completely tested.

exynos_smc(SMC_CMD_CPU1BOOT, ...) is cpu hotplug SMC interface.
Exynos4212 is dual core processor.
Exynos4212 only have to boot cpu1 on smp boot.
So, Second parameter of exynos_smc is fixed by 0 which means cpu1.
It don't need to boot another cpu (ex. cpu2, cpu3 for quad core processor).
But In case of quad core processor (ex. Exynos4412), 
It need to boot another cpu and specify parameter of booting core.
As I know, Exynos3250 is dual core. 
So It can be included 1st condition too.

Sangbeom,
Thanks,
Tomasz Figa April 11, 2014, 7:14 a.m. UTC | #4
Hi Sangbeom,

On 11.04.2014 08:04, Sangbeom Kim wrote:
> Hi,
> On 04/11/2014 2:14 PM, Chanwoo Choi wrote:
>
>>>>   {
>>>> -	exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
>>>> +	if (soc_is_exynos4212())
>>>> +		exynos_smc(SMC_CMD_CPU1BOOT, 0, 0, 0);
>>>> +	else
>>>> +		exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
>>>
>>> 	/* <explain why you need this special case on 4212> */
>>
>> It's better to ask system lsi person. We don't know it well.
>> I got the guide about secondary boot from system lsi.
>> But, this patch was completely tested.
>
> exynos_smc(SMC_CMD_CPU1BOOT, ...) is cpu hotplug SMC interface.
> Exynos4212 is dual core processor.
> Exynos4212 only have to boot cpu1 on smp boot.
> So, Second parameter of exynos_smc is fixed by 0 which means cpu1.
> It don't need to boot another cpu (ex. cpu2, cpu3 for quad core processor).
> But In case of quad core processor (ex. Exynos4412),
> It need to boot another cpu and specify parameter of booting core.
> As I know, Exynos3250 is dual core.
> So It can be included 1st condition too.

Is the smc API defined to ignore the first argument of SMC_CMD_CPU1BOOT 
command for dual core systems or it is defined as should be zero?

Best regards,
Tomasz
diff mbox

Patch

diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
index 932129e..91a911d 100644
--- a/arch/arm/mach-exynos/firmware.c
+++ b/arch/arm/mach-exynos/firmware.c
@@ -18,6 +18,8 @@ 
 
 #include <mach/map.h>
 
+#include <plat/cpu.h>
+
 #include "smc.h"
 
 static int exynos_do_idle(void)
@@ -28,14 +30,22 @@  static int exynos_do_idle(void)
 
 static int exynos_cpu_boot(int cpu)
 {
-	exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
+	if (soc_is_exynos4212())
+		exynos_smc(SMC_CMD_CPU1BOOT, 0, 0, 0);
+	else
+		exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
 	return 0;
 }
 
 static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
 {
-	void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
+	void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c;
 
+	if (soc_is_exynos4212())
+		goto out;
+	else
+		boot_reg += 4*cpu;
+out:
 	__raw_writel(boot_addr, boot_reg);
 	return 0;
 }