Hi Scott,

> On Mon, 2016-01-18 at 12:27 +0800, Dongsheng Wang wrote:
> > From: Wang Dongsheng <dongsheng.w...@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.w...@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.
And this table also contains the sub-feature mask of PSCI functions.

> > +
> > +.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.

> 
> 
> > +
> >  #define AFFINITY_LEVEL_PROCESSOR_SHIFT             8
> >  @ Expect target CPU in r1, return the target cpu number in R0
> >  .globl     psci_get_target_cpu_id
> > @@ -128,6 +181,57 @@ psci_cpu_off:
> >  1: wfi
> >     b       1b
> >
> > +.globl     psci_cpu_suspend
> > +psci_cpu_suspend:
> > +   mov     r0, #PSCI_RET_INVALID_PARAMS
> > +   bx      lr
> > +
> > +.globl     psci_affinity_info
> > +psci_affinity_info:
> > +   push    {lr}
> > +
> > +   mov     r0, #PSCI_RET_INVALID_PARAMS
> > +
> > +   @ Verify Affinity level
> > +   cmp     r2, #0
> > +   bne     out_affinity_info
> > +
> > +   bl      psci_get_target_cpu_id
> > +   cmp     r0, #PSCI_RET_INVALID_PARAMS
> > +   beq     out_affinity_info
> > +   mov     r1, r0
> > +
> > +   @ Get RCPM base address
> > +   movw    r4, #(CONFIG_SYS_FSL_RCPM_ADDR & 0xffff)
> > +   movt    r4, #(CONFIG_SYS_FSL_RCPM_ADDR >> 16)
> > +
> > +   mov     r0, #PSCI_AFFINITY_LEVEL_ON
> > +
> > +   @ Detect target CPU state
> > +   ldr     r2, [r4, #RCPM_TWAITSR]
> > +   rev     r2, r2
> > +   lsr     r2, r2, r1
> > +   ands    r2, r2, #1
> > +   beq     out_affinity_info
> > +
> > +   mov     r0, #PSCI_AFFINITY_LEVEL_OFF
> > +
> > +out_affinity_info:
> > +   pop     {pc}
> 
> Where do you check for ON_PENDING?
> 

Add ON_PENDING in next patch version.

Regards,
-Dongsheng
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to