From e1b27425ecb407ba2a4985be5d83d95dfb10b7ac Mon Sep 17 00:00:00 2001 From: Leo Yan Date: Wed, 3 Jan 2018 14:52:19 +0800 Subject: [PATCH] Revert "Hikey960: Change to use recommended power state id format" This reverts commit fdae60b6ba27c216fd86d13b7432a1ff4f57dd84. The commit fdae60b6ba27c216fd86d13b7432a1ff4f57dd84 changed the parameter encoding for the hikey960. However that implies a DT change in the kernel side. After submitting the DT change for upstreaming, the backward compatibility issue and the interface change raise some concerns from the Linux community about the issues related to kernel <-> ATF alignment. There is no way to detect a mis-alignment of those without a deep knowledge of the ATF and the kernel. Furthermore, the failing calls to PSCI in the idle path (because of bad parameters), will lead to busy looping, implying: thermal issues and extra energy consumption. In regard of the Linux community concerns, the potential issues when the ATF and the kernel are not aligned, it is preferable to revert the commit. Cc: Vincent Guittot Cc: Haojian Zhuang Cc: Kevin Wang Co-authored-by: Daniel Lezcano Signed-off-by: Leo Yan --- plat/hisilicon/hikey960/hikey960_pm.c | 87 ++++++------------- .../hisilicon/hikey960/include/platform_def.h | 6 +- 2 files changed, 29 insertions(+), 64 deletions(-) diff --git a/plat/hisilicon/hikey960/hikey960_pm.c b/plat/hisilicon/hikey960/hikey960_pm.c index 078f0d813..7cbf301c4 100644 --- a/plat/hisilicon/hikey960/hikey960_pm.c +++ b/plat/hisilicon/hikey960/hikey960_pm.c @@ -26,38 +26,6 @@ #define SYSTEM_PWR_STATE(state) \ ((state)->pwr_domain_state[PLAT_MAX_PWR_LVL]) -#define PSTATE_WIDTH 4 -#define PSTATE_MASK ((1 << PSTATE_WIDTH) - 1) - -#define MAKE_PWRSTATE(lvl2_state, lvl1_state, lvl0_state, pwr_lvl, type) \ - (((lvl2_state) << (PSTATE_ID_SHIFT + PSTATE_WIDTH * 2)) | \ - ((lvl1_state) << (PSTATE_ID_SHIFT + PSTATE_WIDTH)) | \ - ((lvl0_state) << (PSTATE_ID_SHIFT)) | \ - ((pwr_lvl) << PSTATE_PWR_LVL_SHIFT) | \ - ((type) << PSTATE_TYPE_SHIFT)) - -/* - * The table storing the valid idle power states. Ensure that the - * array entries are populated in ascending order of state-id to - * enable us to use binary search during power state validation. - * The table must be terminated by a NULL entry. - */ -const unsigned int hikey960_pwr_idle_states[] = { - /* State-id - 0x001 */ - MAKE_PWRSTATE(PLAT_MAX_RUN_STATE, PLAT_MAX_RUN_STATE, - PLAT_MAX_STB_STATE, MPIDR_AFFLVL0, PSTATE_TYPE_STANDBY), - /* State-id - 0x002 */ - MAKE_PWRSTATE(PLAT_MAX_RUN_STATE, PLAT_MAX_RUN_STATE, - PLAT_MAX_RET_STATE, MPIDR_AFFLVL0, PSTATE_TYPE_STANDBY), - /* State-id - 0x003 */ - MAKE_PWRSTATE(PLAT_MAX_RUN_STATE, PLAT_MAX_RUN_STATE, - PLAT_MAX_OFF_STATE, MPIDR_AFFLVL0, PSTATE_TYPE_POWERDOWN), - /* State-id - 0x033 */ - MAKE_PWRSTATE(PLAT_MAX_RUN_STATE, PLAT_MAX_OFF_STATE, - PLAT_MAX_OFF_STATE, MPIDR_AFFLVL1, PSTATE_TYPE_POWERDOWN), - 0, -}; - #define DMAC_GLB_REG_SEC 0x694 #define AXI_CONF_BASE 0x820 @@ -68,21 +36,16 @@ static void hikey960_pwr_domain_standby(plat_local_state_t cpu_state) unsigned long scr; unsigned int val = 0; - assert(cpu_state == PLAT_MAX_STB_STATE || - cpu_state == PLAT_MAX_RET_STATE); + assert(cpu_state == PLAT_MAX_RET_STATE); scr = read_scr_el3(); /* Enable Physical IRQ and FIQ to wake the CPU*/ write_scr_el3(scr | SCR_IRQ_BIT | SCR_FIQ_BIT); - if (cpu_state == PLAT_MAX_RET_STATE) - set_retention_ticks(val); - + set_retention_ticks(val); wfi(); - - if (cpu_state == PLAT_MAX_RET_STATE) - clr_retention_ticks(val); + clr_retention_ticks(val); /* * Restore SCR to the original value, synchronisazion of @@ -161,34 +124,38 @@ static void __dead2 hikey960_system_reset(void) int hikey960_validate_power_state(unsigned int power_state, psci_power_state_t *req_state) { - unsigned int state_id; + unsigned int pstate = psci_get_pstate_type(power_state); + unsigned int pwr_lvl = psci_get_pstate_pwrlvl(power_state); int i; assert(req_state); - /* - * Currently we are using a linear search for finding the matching - * entry in the idle power state array. This can be made a binary - * search if the number of entries justify the additional complexity. - */ - for (i = 0; !!hikey960_pwr_idle_states[i]; i++) { - if (power_state == hikey960_pwr_idle_states[i]) - break; - } - - /* Return error if entry not found in the idle state array */ - if (!hikey960_pwr_idle_states[i]) + if (pwr_lvl > PLAT_MAX_PWR_LVL) return PSCI_E_INVALID_PARAMS; - i = 0; - state_id = psci_get_pstate_id(power_state); - - /* Parse the State ID and populate the state info parameter */ - while (state_id) { - req_state->pwr_domain_state[i++] = state_id & PSTATE_MASK; - state_id >>= PSTATE_WIDTH; + /* Sanity check the requested state */ + if (pstate == PSTATE_TYPE_STANDBY) { + /* + * It's possible to enter standby only on power level 0 + * Ignore any other power level. + */ + if (pwr_lvl != MPIDR_AFFLVL0) + return PSCI_E_INVALID_PARAMS; + + req_state->pwr_domain_state[MPIDR_AFFLVL0] = + PLAT_MAX_RET_STATE; + } else { + for (i = MPIDR_AFFLVL0; i <= pwr_lvl; i++) + req_state->pwr_domain_state[i] = + PLAT_MAX_OFF_STATE; } + /* + * We expect the 'state id' to be zero. + */ + if (psci_get_pstate_id(power_state)) + return PSCI_E_INVALID_PARAMS; + return PSCI_E_SUCCESS; } diff --git a/plat/hisilicon/hikey960/include/platform_def.h b/plat/hisilicon/hikey960/include/platform_def.h index 2ac7f2a75..cb7609076 100644 --- a/plat/hisilicon/hikey960/include/platform_def.h +++ b/plat/hisilicon/hikey960/include/platform_def.h @@ -31,10 +31,8 @@ #define PLAT_NUM_PWR_DOMAINS (PLATFORM_CORE_COUNT + \ PLATFORM_CLUSTER_COUNT + 1) -#define PLAT_MAX_RUN_STATE 0 -#define PLAT_MAX_STB_STATE 1 -#define PLAT_MAX_RET_STATE 2 -#define PLAT_MAX_OFF_STATE 3 +#define PLAT_MAX_RET_STATE 1 +#define PLAT_MAX_OFF_STATE 2 #define MAX_IO_DEVICES 3 #define MAX_IO_HANDLES 4