[U-Boot] [PATCH 8/9] ARM: ARMv7: PSCI: ls102xa: add psci functions implemention

Dongsheng Wang dongsheng.wang at nxp.com
Thu Mar 3 04:54:58 CET 2016


+Zhang Hongbo.

> 
> Hi Scott,
> 
> Thanks for your review.
> 
> > On Tue, 2016-01-19 at 06:28 +0000, Dongsheng Wang wrote:
> > > Hi Scott,
> > >
> > > > On Mon, 2016-01-18 at 12:27 +0800, Dongsheng Wang wrote:
> > > > > From: Wang Dongsheng <dongsheng.wang at nxp.com>
> > > > >
> > > > > Based on PSCI v1.0, implement interface for ls102xa SoC:
> > > > > psci_version,
> > > > > psci_features,
> > > > > psci_cpu_suspend,
> > > > > psci_affinity_info,
> > > > > psci_system_reset,
> > > > > psci_system_off.
> > > > >
> > > > > Tested on LS1021aQDS, LS1021aTWR.
> > > > >
> > > > > Signed-off-by: Wang Dongsheng <dongsheng.wang at nxp.com>
> > > > > ---
> > > > >  arch/arm/cpu/armv7/ls102xa/psci.S          | 110
> > > > > +++++++++++++++++++++++++++--
> > > > >  arch/arm/include/asm/arch-ls102xa/config.h |   1 +
> > > > >  board/freescale/ls1021aqds/Makefile        |   1 +
> > > > >  board/freescale/ls1021aqds/psci.S          |  36 ++++++++++
> > > > >  board/freescale/ls1021atwr/Makefile        |   1 +
> > > > >  board/freescale/ls1021atwr/psci.S          |  28 ++++++++
> > > > >  include/configs/ls1021aqds.h               |   3 +
> > > > >  include/configs/ls1021atwr.h               |   1 +
> > > > >  8 files changed, 177 insertions(+), 4 deletions(-)  create mode
> > > > > 100644 board/freescale/ls1021aqds/psci.S  create mode 100644
> > > > > board/freescale/ls1021atwr/psci.S
> > > > >
> > > > > diff --git a/arch/arm/cpu/armv7/ls102xa/psci.S
> > > > > b/arch/arm/cpu/armv7/ls102xa/psci.S
> > > > > index 3091362..bfc908e 100644
> > > > > --- a/arch/arm/cpu/armv7/ls102xa/psci.S
> > > > > +++ b/arch/arm/cpu/armv7/ls102xa/psci.S
> > > > > @@ -12,19 +12,72 @@
> > > > >  #include <asm/arch-armv7/generictimer.h>  #include <asm/psci.h>
> > > > >
> > > > > +#define RCPM_TWAITSR		0x04C
> > > > > +
> > > > >  #define SCFG_CORE0_SFT_RST      0x130
> > > > >  #define SCFG_CORESRENCR         0x204
> > > > >
> > > > > -#define DCFG_CCSR_BRR           0x0E4
> > > > > -#define DCFG_CCSR_SCRATCHRW1    0x200
> > > > > +#define DCFG_CCSR_RSTCR			0x0B0
> > > > > +#define DCFG_CCSR_RSTCR_RESET_REQ	0x2
> > > > > +#define DCFG_CCSR_BRR			0x0E4
> > > > > +#define DCFG_CCSR_SCRATCHRW1		0x200
> > > > > +
> > > > > +#define PSCI_FN_PSCI_VERSION_FEATURE_MASK	0x0
> > > > > +#define PSCI_FN_CPU_SUSPEND_FEATURE_MASK	0x0
> > > > > +#define PSCI_FN_CPU_OFF_FEATURE_MASK		0x0
> > > > > +#define PSCI_FN_CPU_ON_FEATURE_MASK		0x0
> > > > > +#define PSCI_FN_AFFINITY_INFO_FEATURE_MASK	0x0
> > > > > +#define PSCI_FN_SYSTEM_OFF_FEATURE_MASK		0x0
> > > > > +#define PSCI_FN_SYSTEM_RESET_FEATURE_MASK	0x0
> > > > >
> > > > >  	.pushsection ._secure.text, "ax"
> > > > >
> > > > >  	.arch_extension sec
> > > > >
> > > > > +	.align	5
> > > > > +
> > > > >  #define	ONE_MS		(GENERIC_TIMER_CLK / 1000)
> > > > >  #define	RESET_WAIT	(30 * ONE_MS)
> > > > >
> > > > > +.globl	psci_version
> > > > > +psci_version:
> > > > > +	movw	r0, #0
> > > > > +	movt	r0, #1
> > > > > +
> > > > > +	bx	lr
> > > > > +
> > > > > +_ls102x_psci_supported_table:
> > > > > +	.word	PSCI_FN_PSCI_VERSION
> > > > > +	.word	PSCI_FN_PSCI_VERSION_FEATURE_MASK
> > > > > +	.word	PSCI_FN_CPU_SUSPEND
> > > > > +	.word	PSCI_FN_CPU_SUSPEND_FEATURE_MASK
> > > > > +	.word	PSCI_FN_CPU_OFF
> > > > > +	.word	PSCI_FN_CPU_OFF_FEATURE_MASK
> > > > > +	.word	PSCI_FN_CPU_ON
> > > > > +	.word	PSCI_FN_CPU_ON_FEATURE_MASK
> > > > > +	.word	PSCI_FN_AFFINITY_INFO
> > > > > +	.word	PSCI_FN_AFFINITY_INFO_FEATURE_MASK
> > > > > +	.word	PSCI_FN_SYSTEM_OFF
> > > > > +	.word	PSCI_FN_SYSTEM_OFF_FEATURE_MASK
> > > > > +	.word	PSCI_FN_SYSTEM_RESET
> > > > > +	.word	PSCI_FN_SYSTEM_RESET_FEATURE_MASK
> > > > > +	.word	0
> > > > > +	.word	PSCI_RET_NOT_SUPPORTED
> > > >
> > > > Can you use the main _psci_table instead of duplicating it?
> > > >
> > >
> > > The main table does not apply here. Because this table shows what is
> > > supported in our platform.
> >
> > How does that set differ from what's in the main table?
> >
> 
> The main table include all of functions about PSCI spec. If we use main table to
> match the pass in parameter, it will always return SUPPORT.
> 
> This table just we supported table, if the parameter not matched in this table it
> will finally return NOT_SUPPORTED.
> 
> The main table include in functions ID and function handle, not match this
> feature return value, we not need to get the functions handler, just need to
> return sub-feature, and this table will return sub-feature if the PSCI ID be
> matched.
> 
> > > And this table also contains the sub-feature mask of PSCI functions.
> >
> > ...which is always zero.  As of PSCI 1.0 there's only one function
> > that supports subfeatures, and you could put an explicit check in for
> > that if it ever needs a non-zero value.
> >
> 
> Yes, for now there is only one function that supports sub-features. But this
> table is easy to extend, even if PSCI defines some other sub-features we just
> need to take care this table, and not need to modify the code logic. :)
> 
> > > > > +
> > > > > +.globl	psci_features
> > > > > +psci_features:
> > > > > +	adr	r2, _ls102x_psci_supported_table
> > > > > +1:	ldr	r3, [r2]
> > > > > +	cmp	r3, #0
> > > > > +	beq	out_psci_features
> > > > > +	cmp	r1, r3
> > > > > +	addne	r2, r2, #8
> > > > > +	bne	1b
> > > >
> > > > Why are you adding 8 here?
> > > >
> > >
> > > +4 is the sub-feature mask of the PSCI function. So we need to +8 to
> > > +jump to
> > > next PSCI function.
> > > .word	PSCI_FN_PSCI_VERSION
> > > .word	PSCI_FN_PSCI_VERSION_FEATURE_MASK
> > >
> > > > > +
> > > > > +out_psci_features:
> > > > > +	ldr	r0, [r2, #4]
> > > > > +	bx	lr
> > > >
> > > > If you find a match, you're supposed to return zero, not the next
> > > > function id in the table.
> > > > How did you test this?  There should really be a test suite for
> > > > runtime services such as this, especially when trying to comply
> > > > with a standard.
> > >
> > > I think maybe you missed something about this code. The return value
> > > is PSCI_FN_PSCI_XXXXXX_FEATURE_MASK, not return next function ID.
> >
> > Yes, I misread the table and missed the masks.  But see above about
> > them being unnecessary.
> >
> > In any case, a test suite would be very helpful.
> >
> Yes, thanks for your friendly reminder. And this table has tested on ls1021
> platform. :)
> 
> Regards,
> -Dongsheng



More information about the U-Boot mailing list