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 <vincent.guittot@linaro.org>
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Cc: Kevin Wang <jean.wangtao@linaro.org>
Co-authored-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>

diff --git a/plat/hisilicon/hikey960/hikey960_pm.c b/plat/hisilicon/hikey960/hikey960_pm.c
index 078f0d81344f98ef2241276045c210a226231025..7cbf301c46934cc51470c4806ea7f1c6ba6851c3 100644 (file)
--- 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 2ac7f2a75c265d7bcc2fd9a0cb61b2a8b2e1f6f4..cb7609076cda342dffe3c56f75b65a06038398cf 100644 (file)
--- 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