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

[Tom Rini]

On Thu, Mar 29, 2018 at 04:59:21PM +0000, Patrick DELAUNAY wrote:
> Hi  Mark,
> 
> 
> > -----Original Message-----
> > 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>
> > 
> > > +int __secure psci_features(unsigned int psci_fid) {
> > > +	switch (psci_fid) {
> > > +	case ARM_PSCI_0_2_FN_PSCI_VERSION:
> > > +	case ARM_PSCI_0_2_FN_CPU_ON:
> > > +	case ARM_PSCI_0_2_FN_CPU_OFF:
> > > +	case ARM_PSCI_0_2_FN_SYSTEM_RESET:
> > > +		return 0x0;
> > > +	}
> > > +	return ARM_PSCI_RET_NI;
> > > +}
> > >
> > > +unsigned int __secure psci_version(void) {
> > > +	return ARM_PSCI_VER_1_0;
> > > +}
> > 
> > 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180401/03471278/attachment.sig>