[U-Boot] [PATCH v5 2/8] ARMv7: PSCI: factor out reusable psci_cpu_on_common

[Hongbo Zhang]

On Tue, Jun 28, 2016 at 10:49 AM, Chen-Yu Tsai <wens at csie.org> wrote:
> Hi,
>
> On Tue, Jun 14, 2016 at 3:01 PM,  <macro.wave.z at gmail.com> wrote:
>> From: Hongbo Zhang <hongbo.zhang at nxp.com>
>>
>> There are codes for saving target PC in each platform psci_cpu_on routines,
>> these can be factored out as psci_cpu_on_common.
>> Another purpose is that the codes of saving target PC will be changed a bit and
>> a new context ID will be saved in the following patches, then we can update the
>> psci_cpu_on_common mainly instead of touching every platform's psci_cpu_on
>> functions more times, this makes it easier for coding and being reviewed.
>>
>> Signed-off-by: Hongbo Zhang <hongbo.zhang at nxp.com>
>> Signed-off-by: Wang Dongsheng <dongsheng.wang at nxp.com>
>> ---
>>  arch/arm/cpu/armv7/ls102xa/psci.S     |  5 +----
>>  arch/arm/cpu/armv7/mx7/psci.S         |  5 +----
>>  arch/arm/cpu/armv7/psci.S             | 12 ++++++++++++
>>  arch/arm/cpu/armv7/sunxi/psci_sun6i.S |  5 +----
>>  arch/arm/cpu/armv7/sunxi/psci_sun7i.S |  5 +----
>>  arch/arm/mach-tegra/psci.S            |  5 +----
>>  6 files changed, 17 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/ls102xa/psci.S b/arch/arm/cpu/armv7/ls102xa/psci.S
>> index cf5cd48..548c507 100644
>> --- a/arch/arm/cpu/armv7/ls102xa/psci.S
>> +++ b/arch/arm/cpu/armv7/ls102xa/psci.S
>> @@ -35,10 +35,7 @@ psci_cpu_on:
>>         @ r1 = 0xf01
>>         and     r1, r1, #0xff
>>
>> -       mov     r0, r1
>> -       bl      psci_get_cpu_stack_top
>> -       str     r2, [r0]
>> -       dsb
>> +       bl      psci_cpu_on_common
>>
>>         @ Get DCFG base address
>>         movw    r4, #(CONFIG_SYS_FSL_GUTS_ADDR & 0xffff)
>> diff --git a/arch/arm/cpu/armv7/mx7/psci.S b/arch/arm/cpu/armv7/mx7/psci.S
>> index 34c6ab3..74fdc4d 100644
>> --- a/arch/arm/cpu/armv7/mx7/psci.S
>> +++ b/arch/arm/cpu/armv7/mx7/psci.S
>> @@ -29,10 +29,7 @@ psci_arch_init:
>>  psci_cpu_on:
>>         push    {lr}
>>
>> -       mov     r0, r1
>> -       bl      psci_get_cpu_stack_top
>> -       str     r2, [r0]
>> -       dsb
>> +       bl      psci_cpu_on_common
>>
>>         ldr     r2, =psci_cpu_entry
>>         bl      imx_cpu_on
>> diff --git a/arch/arm/cpu/armv7/psci.S b/arch/arm/cpu/armv7/psci.S
>> index 8e25300..3b92f1d 100644
>> --- a/arch/arm/cpu/armv7/psci.S
>> +++ b/arch/arm/cpu/armv7/psci.S
>> @@ -253,6 +253,18 @@ ENTRY(psci_enable_smp)
>>  ENDPROC(psci_enable_smp)
>>  .weak psci_enable_smp
>>
>> +/* expects target CPU in r1, target PC in r2 */
>> +ENTRY(psci_cpu_on_common)
>> +       push    {lr}
>> +
>> +       mov     r0, r1
>> +       bl      psci_get_cpu_stack_top          @ get stack top of target CPU
>> +       str     r2, [r0]                        @ store target PC at stack top
>> +       dsb
>> +
>> +       pop     {pc}
>> +ENDPROC(psci_cpu_on_common)
>> +
>
> Since you are just saving the target PC, why not just name the
> function like that?
>

No, not only saving target PC, you will see later that the newly added
context ID is saved too.
In my previous patches, I save target PC here, and then in later patch
save target ID too, and at last the common function is factored out,
in this way every platform's code are touched too many times.

Copy one paragraph of my cover letter here:
"Move the patch of factoring out psci_cpu_on_common to earlier place 2/8, so
that the following patches can only update the psci_cpu_on_common instead of
touching each platform's psci_cpu_on, this reduces patches size obviously and
make it easier for review."

> See https://github.com/wens/u-boot-sunxi/commit/5a42630107e93bea9aa4da27cc16bdb604993dca
>
>>  ENTRY(psci_cpu_off_common)
>>         push    {lr}
>>
>> diff --git a/arch/arm/cpu/armv7/sunxi/psci_sun6i.S b/arch/arm/cpu/armv7/sunxi/psci_sun6i.S
>> index 90b5bfd..016e491 100644
>> --- a/arch/arm/cpu/armv7/sunxi/psci_sun6i.S
>> +++ b/arch/arm/cpu/armv7/sunxi/psci_sun6i.S
>> @@ -134,10 +134,7 @@ out:       mcr     p15, 0, r7, c1, c1, 0
>>  psci_cpu_on:
>>         push    {lr}
>>
>> -       mov     r0, r1
>> -       bl      psci_get_cpu_stack_top  @ get stack top of target CPU
>> -       str     r2, [r0]                @ store target PC at stack top
>> -       dsb
>> +       bl      psci_cpu_on_common
>>
>>         movw    r0, #(SUN6I_CPUCFG_BASE & 0xffff)
>>         movt    r0, #(SUN6I_CPUCFG_BASE >> 16)
>> diff --git a/arch/arm/cpu/armv7/sunxi/psci_sun7i.S b/arch/arm/cpu/armv7/sunxi/psci_sun7i.S
>> index e15d587..0ebb30e 100644
>> --- a/arch/arm/cpu/armv7/sunxi/psci_sun7i.S
>> +++ b/arch/arm/cpu/armv7/sunxi/psci_sun7i.S
>> @@ -123,10 +123,7 @@ out:       mcr     p15, 0, r7, c1, c1, 0
>>  psci_cpu_on:
>>         push    {lr}
>>
>> -       mov     r0, r1
>> -       bl      psci_get_cpu_stack_top  @ get stack top of target CPU
>> -       str     r2, [r0]                @ store target PC at stack top
>> -       dsb
>> +       bl      psci_cpu_on_common
>
> The sunxi parts no longer apply. The partial rewrite in C has
> landed in master.
>
OK, will check sunxi's codes again.

> Regards
> ChenYu
>
>>
>>         movw    r0, #(SUN7I_CPUCFG_BASE & 0xffff)
>>         movt    r0, #(SUN7I_CPUCFG_BASE >> 16)
>> diff --git a/arch/arm/mach-tegra/psci.S b/arch/arm/mach-tegra/psci.S
>> index b836da1..8a0147c 100644
>> --- a/arch/arm/mach-tegra/psci.S
>> +++ b/arch/arm/mach-tegra/psci.S
>> @@ -90,10 +90,7 @@ ENDPROC(psci_cpu_off)
>>  ENTRY(psci_cpu_on)
>>         push    {lr}
>>
>> -       mov     r0, r1
>> -       bl      psci_get_cpu_stack_top  @ get stack top of target CPU
>> -       str     r2, [r0]                @ store target PC at stack top
>> -       dsb
>> +       bl      psci_cpu_on_common
>>
>>         ldr     r6, =TEGRA_RESET_EXCEPTION_VECTOR
>>         ldr     r5, =psci_cpu_entry
>> --
>> 2.1.4
>>