[U-Boot] [PATCH 2/3] ARM: rmobile: Add basic PSCI support for r8a7790 SoC

[Oleksandr]

Hi, Marek


>>>>>>> +}
>>>>>>> +
>>>>>>> +/*
>>>>>>> + * Reset vector for secondary CPUs.
>>>>>>> + * This will be mapped at address 0 by SBAR register.
>>>>>>> + * We need _long_ jump to the physical address.
>>>>>>> + */
>>>>>>> +asm("    .arm\n"
>>>>>>> +    "    .align 12\n"
>>>>>>> +    "    .globl shmobile_boot_vector\n"
>>>>>>> +    "shmobile_boot_vector:\n"
>>>>>>> +    "    ldr r1, 1f\n"
>>>>>>> +    "    bx    r1\n"
>>>>>>> +    "    .type shmobile_boot_vector, %function\n"
>>>>>>> +    "    .size shmobile_boot_vector, .-shmobile_boot_vector\n"
>>>>>>> +    "    .align    2\n"
>>>>>>> +    "    .globl    shmobile_boot_fn\n"
>>>>>>> +    "shmobile_boot_fn:\n"
>>>>>>> +    "1:    .space    4\n"
>>>>>>> +    "    .globl    shmobile_boot_size\n"
>>>>>>> +    "shmobile_boot_size:\n"
>>>>>>> +    "    .long    .-shmobile_boot_vector\n");
>>>>>> Why can't this be implemented in C ?
>>>>> This "reset vector" code was ported from Linux:
>>>>>
>>>>> https://elixir.bootlin.com/linux/v5.0-rc5/source/arch/arm/mach-shmobile/headsmp.S#L21 
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Really don't know whether it can be implemented in C.
>>>>>
>>>>> I was thinking of moving this code to a separate ASM file in order 
>>>>> not
>>>>> to mix C and ASM. What do you think about it?
>>>> U-Boot already has a reset vector code, can't that be reused ?
>>> I don't think. Being honest, I couldn't find an obvious way how to 
>>> reuse
>>> (I assume you meant arch/arm/cpu/armv7/start.S).
>> Maybe it needs some additional work first ?
>> It seems Altera socfpga somehow uses the U-Boot reset vectors for PSCI,
>> so it should at least be possible.
>
> Could you, please, point me in code? Unfortunately, I wasn't able to 
> find.
>
>
>>
>>> The newly turned on secondary CPU entry should be common
>>> "psci_cpu_entry", which does proper things.
>>>
>>> And this reset vector is just "a small piece of code" to be located in
>>> on-chip RAM (with limited size) and used for the jump stub...
>> We already have the SPL reset vectors in SRAM, maybe that can be
>> recycled somehow ?
>
>
> The only idea I have, how it may be recycled (not sure whether it will 
> work...)
>
>
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index 0cb6dd39cc..69acf4677b 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
> @@ -36,6 +36,12 @@
>  #endif
>
>  reset:
> +
> +#if defined(CONFIG_ARMV7_PSCI) && !defined(CONFIG_SPL_BUILD)
> +       b psci_cpu_entry_jump
> +       /* return only if this is not "a newly turned on CPU" using 
> PSCI) */
> +#endif
> +
>         /* Allow the board to save important registers */
>         b       save_boot_params
>  save_boot_params_ret:
> @@ -128,6 +134,21 @@ ENDPROC(switch_to_hypervisor)
>         .weak   switch_to_hypervisor
>  #endif
>
> +/*
> + * Each platform which implements psci_cpu_entry_jump function should 
> perform
> + * in the following way:
> + *
> + * If the executing this call CPU is exactly that CPU we are 
> expecting to be
> + * powered on, then jump to psci_cpu_entry and never return.
> + * Otherwise return to the caller.
> + */
> +#if defined(CONFIG_ARMV7_PSCI) && !defined(CONFIG_SPL_BUILD)
> +ENTRY(psci_cpu_entry_jump)
> +       movs    pc, lr
> +ENDPROC(psci_cpu_entry_jump)
> +.weak psci_cpu_entry_jump
> +#endif
> +
>  /************************************************************************* 
>
>   *
>   * cpu_init_cp15
>
>
> What do you think?
>
>
> It would be much appreciated, if you could provide some hints.


Don't want to be annoying, but it would be really nice to get my 
questions answered...


-- 
Regards,

Oleksandr Tyshchenko