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

[Oleksandr Tyshchenko]

Hi, Marek

sorry for possible format issue.


ср, 27 февр. 2019 г., 23:02 Marek Vasut <marek.vasut at gmail.com>:

> On 2/26/19 8:37 PM, Oleksandr wrote:
> >
> > Hi, Marek
>
> Hi,
>
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>> +/*
> >>>>>>>> + * 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...
>
> I am sorry, I am too busy right now. I will answer that once I can
> properly study the problem and give you a useful answer.
>

No problem, I will wait.

Thank you.


> --
> Best regards,
> Marek Vasut
>