[U-Boot] [PATCH] arm: stm32mp1: add PSCI support

[Patrick DELAUNAY]

Hi Tom

> From: Tom Rini [mailto:trini at konsulko.com]
> 
> On Thu, Mar 29, 2018 at 04:59:21PM +0000, Patrick DELAUNAY wrote:
> > Hi  Mark,
> >
> >
> > > From: Mark Rutland [mailto:mark.rutland at arm.com]
> > >
> > > Hi,
> > >
> > > On Tue, Mar 20, 2018 at 01:59:03PM +0100, Patrick Delaunay wrote:
> > > > Add minimal PSCI support for Linux.
> > > >
> > > > Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> > >
> > > I'm a bit worried, because while PSCI_VERSION reports PSCI 1.0, this
> > > does not appear to be conformant with teh PSCI 1.0 spec, as some
> > > mandatory functions are missing:
> > >
> > > * AFFINITY_INFO -- Linux relies on this to ensure that CPUs have exited
> > >   the kernel when calling CPU_OFF (e.g. so that we don't free any data /
> > >   code that said CPU may be using on the path to calling CPU_OFF).
> > >
> > > * SYSTEM_OFF -- Can you implement this similarly to SYSTEM_RESET?
> >
> > I push this patch for STM32MP1 to be able to boot the Kernel version
> > on board (requested by ST Linux team :
> > https://patchwork.kernel.org/patch/10167343/)
> >
> > This patch  a minimal PSCI support to be able to boot....
> > And I miss these part of the requirement PSCI v1.0 and also in issue in Linux
> trace:
> >
> > [    0.000000] psci: probing for conduit method from DT.
> > [    0.000000] psci: PSCIv1.0 detected in firmware.
> > [    0.000000] psci: Using standard PSCI v0.2 function IDs
> > [    0.000000] psci: MIGRATE_INFO_TYPE not supported.
> >
> > Thanks to point these point .
> > So I will add them in a second patch " arm: stm32mp1: complete PSCI v1.0
> support"
> >
> > I want keep this patch in v1, to have it merged in v2018.05-rc1, I hope...
> > Even if it is only a first step for a full PSCI v1.0 support, that allow linux kernel
> boot.
> >
> > And I need some time to check how to handle SYSTEM_OFF , to do the
> "completely removes power" because I don't think if it is possible today.
> >
> > > > +int __secure psci_cpu_on(u32 __always_unused unused, u32 mpidr,
> > > > +u32
> > > > +pc) {
> > >
> > > What about the context_id? PSCI 0.2+ mandates it, and some project
> > > rely upon it (though Linux currently does not).
> >
> > Yes context_id is not used in my code.....
> >
> > In fact, I use the same psci_cpu_on prototypes and logic than other platform :
> >  ./arch/arm/cpu/armv7/sunxi/psci.c:246
> > ./arch/arm/mach-uniphier/arm32/psci.c:134
> > ./arch/arm/cpu/armv7/ls102xa/psci.S:117
> >
> > => all are based on psci_save_target_pc() to save the PC in secure context
> (psci_target_pc[])
> > 	./arch/arm/cpu/armv7/psci-common.c:29
> >
> > But context is not saved or used in generic PSCI code, only pc is restored
> > 	=> ./arch/arm/cpu/armv7/psci.S:330
> > 		r0 = psci_get_target_pc() is called before _do_nonsec_entry
> >
> > I will check deeper in this U-Boot PSCI code.
> >
> > => I try to solve the problem and push a RFC or a patch  to have feedback form
> other PSCI user.
> >      Even it is not block for Linux point of view, as context ID is not used in linux
> code always = 0:
> >      ./drivers/firmware/psci.c:
> > 	static int psci_cpu_on(unsigned long cpuid, unsigned long entry_point)
> > 	....
> > 		err = invoke_psci_fn(fn, cpuid, entry_point, 0);
> >
> > > Does some other part of U-Boot do some state tracking? What happens
> > > if this is called for a CPU that's already online?
> >
> > No issue,  the CPU continue to run....
> > TAMP register is only used by BootRom to unpark the cpu on boot, update of
> the backup have no impact when the CPU is running.
> >
> > I will add a comment in the next patch " arm: stm32mp1: complete PSCI v1.0
> support"
> > and also check if I can add test the cpu state to manage other possible return
> value (never used in U-Boot):
> > 	PSCI_RET_ALREADY_ON
> > 	PSCI_RET_ON_PENDING
> >
> > >
> > > Thanks,
> > > Mark.
> >
> > Very thanks for the review.
> > That allows me to read deeper the PSCI documents.
> >
> > Tom:  do you think that this patch can be merged in v2018.05-rc1 ?
> >            and I will sent a 2nd patch for full PCSI v1.0 support and correct the
> missing parts reported by Mark.
> 
> Lets see how far along you get before the v2018.05 final.  I don't think it's a
> good idea to claim PSCI 1.0 support and not provide all of the required
> functionality, as that would be breaking the point of declaring that you support
> something like that :)  Thanks!
> 
> --
> Tom

I understood and I am agree that it is better, 
So I will push a v2 as soon as possible with minimal PSCI v1.0 (adding the 2 missing functions).

And an other patch to save the context ID for PSCI on arm v7 (with perhaps impact on other platform).

Patrick