smcf: Check MODE_STAT after setting MODE_REQ

This patch introduces a check in the MGI module to ensure MODE_REQ registers is set successfully. From the RTL design, MODE_REQ registers should be configured individually and the register has to be processed before setting the next MODE_REQ register. This is done by checking the MODE_STAT register and a timeout is added to prevent indefinite wait. Signed-off-by: Wen Ping Teh <[email protected]> Change-Id: I6fe0469dea9a68f6c7374596b5566cdafeb17fcb
ARM-software · May 10, 2024 · 9860d64 · 9860d64
1 parent f69a536
commit 9860d64
Show file tree

Hide file tree

Showing 10 changed files with 367 additions and 103 deletions.
diff --git a/module/amu_smcf_drv/test/CMakeLists.txt b/module/amu_smcf_drv/test/CMakeLists.txt
@@ -1,6 +1,6 @@
 #
 # Arm SCP/MCP Software
-# Copyright (c) 2023, Arm Limited and Contributors. All rights reserved.
+# Copyright (c) 2023-2024, Arm Limited and Contributors. All rights reserved.
 #
 # SPDX-License-Identifier: BSD-3-Clause
 #
@@ -21,6 +21,7 @@ list(APPEND OTHER_MODULE_INC ${SCP_ROOT}/interface/amu)
 list(APPEND OTHER_MODULE_INC ${MODULE_ROOT}/smcf/include)
 list(APPEND OTHER_MODULE_INC ${MODULE_ROOT}/smcf/src)
 list(APPEND OTHER_MODULE_INC ${MODULE_ROOT}/smcf/test/mocks)
+list(APPEND OTHER_MODULE_INC ${MODULE_ROOT}/timer/include)
 set(MODULE_UT_SRC ${CMAKE_CURRENT_LIST_DIR})
 set(MODULE_UT_INC ${CMAKE_CURRENT_LIST_DIR})
 set(MODULE_UT_MOCK_SRC ${CMAKE_CURRENT_LIST_DIR}/mocks)

diff --git a/module/smcf/include/mod_smcf.h b/module/smcf/include/mod_smcf.h
@@ -1,6 +1,6 @@
 /*
  * Arm SCP/MCP Software
- * Copyright (c) 2023, Arm Limited and Contributors. All rights reserved.
+ * Copyright (c) 2023-2024, Arm Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  *
@@ -31,6 +31,30 @@
 /*! Maximum number of mode entries as defined by the hardware spec. */
 #define SMCF_MODE_ENTRY_COUNT 4
 
+/*!
+ * \brief Timer for setting MODE_REQ register.
+ *
+ * \details This structure is required to be filled in SMCF config file only
+ *          when the timeout feature is required.
+ */
+struct mod_smcf_timer_config {
+    /*!
+     * \brief Timer identifier.
+     *
+     * \details Used for binding with the timer API and waiting for specified
+     *          delay after setting the MODE_REQ register.
+     */
+    fwk_id_t timer_id;
+
+    /*!
+     * \brief Timeout value.
+     *
+     * \details MODE_REQ state change wait delay in micro seconds. A valid
+     *          non-zero value has to be specified when using this feature.
+     */
+    uint32_t set_state_timeout_us;
+};
+
 /*!
  * \brief Configuration data of a domain driver
  */
@@ -46,6 +70,9 @@ struct mod_smcf_element_config {
 
     /*! Data location and header format */
     struct mod_smcf_data_config data_config;
+
+    /*! Timer descriptor */
+    struct mod_smcf_timer_config *timer_config;
 };
 
 /*!

diff --git a/module/smcf/src/mgi.c b/module/smcf/src/mgi.c
@@ -1,6 +1,6 @@
 /*
  * Arm SCP/MCP Software
- * Copyright (c) 2023, Arm Limited and Contributors. All rights reserved.
+ * Copyright (c) 2023-2024, Arm Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
@@ -10,6 +10,11 @@
 
 #include <stddef.h>
 
+struct mgi_set_monitor_mode_check {
+    uint32_t mode_idx;
+    struct smcf_mgi_reg *smcf_mgi;
+};
+
 /* Get the number of monitors supported by this MGI */
 uint32_t mgi_get_num_of_monitors(struct smcf_mgi_reg *smcf_mgi)
 {
@@ -166,20 +171,19 @@ int mgi_disable_program_mode(struct smcf_mgi_reg *smcf_mgi, uint32_t monitor)
     return FWK_SUCCESS;
 }
 
-/* Set MODE for individual MODE_REQ register */
-int mgi_set_monitor_mode(
-    struct smcf_mgi_reg *smcf_mgi,
-    uint32_t mode_idx,
-    uint32_t value)
+bool mgi_is_monitor_mode_updated(void *data)
 {
-    uint32_t mode_registers_num = mgi_get_number_of_mode_registers(smcf_mgi);
-    uint32_t mode_registers_bits =
-        mgi_get_number_of_bits_in_mode_registers(smcf_mgi);
-    uint32_t mode_mask = (1U << mode_registers_bits) - 1;
+    uint32_t mode_registers_num;
+    struct mgi_set_monitor_mode_check *params;
+    struct smcf_mgi_reg *smcf_mgi;
 
-    if (mode_idx > (mode_registers_num - 1)) {
-        return FWK_E_RANGE;
-    }
+    fwk_assert(data != NULL);
+    params = (struct mgi_set_monitor_mode_check *)data;
+    fwk_assert(params->smcf_mgi != NULL);
+    smcf_mgi = params->smcf_mgi;
+
+    mode_registers_num = mgi_get_number_of_mode_registers(smcf_mgi);
+    fwk_assert(params->mode_idx <= (mode_registers_num - 1));
 
     FWK_RW uint32_t *mode_req[SMCF_MGI_MAX_NUM_MODE_REG] = {
         &smcf_mgi->MODE_REQ0,
@@ -188,15 +192,32 @@ int mgi_set_monitor_mode(
         &smcf_mgi->MODE_REQ3
     };
 
-    *mode_req[mode_idx] = (value & mode_mask);
+    FWK_R uint32_t *mode_stat[SMCF_MGI_MAX_NUM_MODE_REG] = {
+        &smcf_mgi->MODE_STAT0,
+        &smcf_mgi->MODE_STAT1,
+        &smcf_mgi->MODE_STAT2,
+        &smcf_mgi->MODE_STAT3
+    };
 
-    return FWK_SUCCESS;
+    return (*mode_req[params->mode_idx] == *mode_stat[params->mode_idx]);
 }
 
-bool mgi_is_monitor_mode_updated(struct smcf_mgi_reg *smcf_mgi)
+/* Set MODE for individual MODE_REQ register */
+int mgi_set_monitor_mode(
+    struct smcf_mgi_reg *smcf_mgi,
+    struct smcf_mgi_timer_ctx *timer_ctx,
+    uint32_t mode_idx,
+    uint32_t value)
 {
-    uint32_t i;
     uint32_t mode_registers_num = mgi_get_number_of_mode_registers(smcf_mgi);
+    uint32_t mode_registers_bits =
+        mgi_get_number_of_bits_in_mode_registers(smcf_mgi);
+    uint32_t mode_mask = (1U << mode_registers_bits) - 1;
+    struct mgi_set_monitor_mode_check params;
+
+    if (mode_idx > (mode_registers_num - 1)) {
+        return FWK_E_RANGE;
+    }
 
     FWK_RW uint32_t *mode_req[SMCF_MGI_MAX_NUM_MODE_REG] = {
         &smcf_mgi->MODE_REQ0,
@@ -205,20 +226,28 @@ bool mgi_is_monitor_mode_updated(struct smcf_mgi_reg *smcf_mgi)
         &smcf_mgi->MODE_REQ3
     };
 
-    FWK_R uint32_t *mode_stat[SMCF_MGI_MAX_NUM_MODE_REG] = {
-        &smcf_mgi->MODE_STAT0,
-        &smcf_mgi->MODE_STAT1,
-        &smcf_mgi->MODE_STAT2,
-        &smcf_mgi->MODE_STAT3
-    };
+    *mode_req[mode_idx] = (value & mode_mask);
 
-    for (i = 0; i < mode_registers_num; i++) {
-        if (*mode_stat[i] != *mode_req[i]) {
-            return false;
-        }
+    if (timer_ctx == NULL) {
+        FWK_R uint32_t *mode_stat[SMCF_MGI_MAX_NUM_MODE_REG] = {
+            &smcf_mgi->MODE_STAT0,
+            &smcf_mgi->MODE_STAT1,
+            &smcf_mgi->MODE_STAT2,
+            &smcf_mgi->MODE_STAT3
+        };
+        while (*mode_req[mode_idx] != *mode_stat[mode_idx])
+            continue;
+    } else {
+        params.mode_idx = mode_idx;
+        params.smcf_mgi = smcf_mgi;
+        return timer_ctx->timer_api->wait(
+            timer_ctx->timer_id,
+            timer_ctx->delay_us,
+            mgi_is_monitor_mode_updated,
+            &params);
     }
 
-    return true;
+    return FWK_SUCCESS;
 }
 
 /*

diff --git a/module/smcf/src/mgi.h b/module/smcf/src/mgi.h
@@ -1,6 +1,6 @@
 /*
  * Arm SCP/MCP Software
- * Copyright (c) 2023, Arm Limited and Contributors. All rights reserved.
+ * Copyright (c) 2023-2024, Arm Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
@@ -12,7 +12,10 @@
  * \cond
  */
 
+#include <mod_timer.h>
+
 #include <fwk_attributes.h>
+#include <fwk_id.h>
 #include <fwk_macros.h>
 #include <fwk_status.h>
 
@@ -207,6 +210,15 @@ struct smcf_mgi_reg {
     FWK_R uint32_t AIDR;
 };
 
+/*
+ * Timer context for MODE_REQ timeout
+ */
+struct smcf_mgi_timer_ctx {
+    fwk_id_t timer_id;
+    struct mod_timer_api *timer_api;
+    uint32_t delay_us;
+};
+
 /*
  * Bit definitions for MGI_GRP_ID
  */
@@ -460,6 +472,7 @@ int mgi_enable__program_mode_multi(
 /* Set MODE for individual MODE_REQ reigester */
 int mgi_set_monitor_mode(
     struct smcf_mgi_reg *smcf_mgi,
+    struct smcf_mgi_timer_ctx *timer_ctx,
     uint32_t mode_idx,
     uint32_t value);
 

diff --git a/module/smcf/src/mod_smcf.c b/module/smcf/src/mod_smcf.c
@@ -49,6 +49,9 @@ struct smcf_element_ctx {
 
     /* Data attributes */
     struct smcf_data_attr data_attr;
+
+    /* Timer context*/
+    struct smcf_mgi_timer_ctx *timer_ctx;
 };
 
 /* Module context */
@@ -290,7 +293,8 @@ static int smcf_mli_config_mode_set(
         return status;
     }
 
-    status = mgi_set_monitor_mode(element_ctx->mgi, mode_index, mode_value);
+    status = mgi_set_monitor_mode(
+        element_ctx->mgi, element_ctx->timer_ctx, mode_index, mode_value);
     if (status != FWK_SUCCESS) {
         return status;
     }
@@ -347,6 +351,7 @@ static int smcf_mod_init(
         fwk_mm_calloc(mgi_count, sizeof(struct smcf_element_ctx));
 
     mod_ctx.element_ctx_table_size = mgi_count;
+
     return FWK_SUCCESS;
 }
 
@@ -447,6 +452,26 @@ static int smcf_element_init(
     }
     ctx->monitor_count = sub_element_count;
 
+#ifdef BUILD_HAS_MOD_TIMER
+    if (config->timer_config == NULL) {
+        ctx->timer_ctx = NULL;
+    } else {
+        ctx->timer_ctx = fwk_mm_calloc(1, sizeof(struct smcf_mgi_timer_ctx));
+        if (ctx->timer_ctx == NULL) {
+            return FWK_E_NOMEM;
+        }
+        /* Check for valid timeout value if timer ID is specified */
+        if (config->timer_config->set_state_timeout_us == 0) {
+            return FWK_E_PARAM;
+        }
+        /* Save the timer ID to context */
+        ctx->timer_ctx->timer_id = config->timer_config->timer_id;
+        ctx->timer_ctx->delay_us = config->timer_config->set_state_timeout_us;
+    }
+#else
+    ctx->timer_ctx = NULL;
+#endif
+
     status = smcf_element_init_config_sample_type(ctx);
     if (status != FWK_SUCCESS) {
         return status;
@@ -530,6 +555,35 @@ static int smcf_process_event(
     return status;
 }
 
+#ifdef BUILD_HAS_MOD_TIMER
+static int smcf_bind(fwk_id_t id, unsigned int round)
+{
+    int status = FWK_SUCCESS;
+    struct smcf_element_ctx *ctx = get_domain_ctx(id);
+
+    /* Only bind in first round of calls. */
+    if (round > 0) {
+        return FWK_SUCCESS;
+    }
+
+    if (fwk_id_is_type(id, FWK_ID_TYPE_MODULE)) {
+        return FWK_SUCCESS;
+    }
+
+    if (ctx->timer_ctx != NULL &&
+        !fwk_id_is_equal(ctx->timer_ctx->timer_id, FWK_ID_NONE)) {
+        /* Bind to the timer */
+        status = fwk_module_bind(
+            ctx->timer_ctx->timer_id,
+            MOD_TIMER_API_ID_TIMER,
+            &ctx->timer_ctx->timer_api);
+        if (status != FWK_SUCCESS) {
+            return status;
+        }
+    }
+}
+#endif
+
 const struct fwk_module module_smcf = {
     .type = FWK_MODULE_TYPE_DRIVER,
     .api_count = (unsigned int)MOD_SMCF_API_IDX_COUNT,
@@ -541,4 +595,7 @@ const struct fwk_module module_smcf = {
     .element_init = smcf_element_init,
     .process_bind_request = smcf_process_bind_request,
     .process_event = smcf_process_event,
+#ifdef BUILD_HAS_MOD_TIMER
+    .bind = smcf_bind,
+#endif
 };
diff --git a/module/smcf/test/mgi/fwk_module_idx.h b/module/smcf/test/mgi/fwk_module_idx.h
@@ -1,6 +1,6 @@
 /*
  * Arm SCP/MCP Software
- * Copyright (c) 2023, Arm Limited and Contributors. All rights reserved.
+ * Copyright (c) 2023-2024, Arm Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
@@ -14,6 +14,7 @@ enum fwk_module_idx {
     FWK_MODULE_IDX_SMCF,
     FWK_MODULE_IDX_SENSOR,
     FWK_MODULE_IDX_COUNT,
+    FWK_MODULE_IDX_TIMER,
 };
 
 #endif /* TEST_FWK_MODULE_MODULE_IDX_H */