From b1ceab919c1157f3dfd27b4d502bd622e71ee515 Mon Sep 17 00:00:00 2001 From: Jim Quigley Date: Wed, 13 May 2020 10:34:17 +0100 Subject: [PATCH 1/6] tools: Fix Python code style errors Release 2.3.1 of pycodestyle does not allow the use of 'l' as a variable. Change-Id: I938515fdc0b6fe3e348995f1b35f0c98b311950e Signed-off-by: Jim Quigley --- tools/check_api.py | 2 +- tools/check_tabs.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/check_api.py b/tools/check_api.py index 2dac9c82d..7427a21da 100755 --- a/tools/check_api.py +++ b/tools/check_api.py @@ -71,7 +71,7 @@ def main(argv=[], prog_name=''): illegal_use = 0 with open(BANNED_LIST) as file: - for l, fname in enumerate(file): + for fname in file: if fname[0] == '#': continue BANNED_API.append(fname.rstrip()) diff --git a/tools/check_tabs.py b/tools/check_tabs.py index 35110c552..2aae02ce6 100755 --- a/tools/check_tabs.py +++ b/tools/check_tabs.py @@ -80,7 +80,7 @@ def main(argv=[], prog_name=''): # excluded by .git/info/exclude) git_clean_output = subprocess.check_output("git clean -ndX".split()) git_clean_output = git_clean_output.decode() - git_ignores = [l.split()[-1] for l in git_clean_output.splitlines()] + git_ignores = [line.split()[-1] for line in git_clean_output.splitlines()] cwd = os.getcwd() print("Executing from %s" % cwd) -- GitLab From 7ad48c0af36a5415725260268ef5d43698a25cd4 Mon Sep 17 00:00:00 2001 From: Jim Quigley Date: Wed, 13 May 2020 13:54:45 +0100 Subject: [PATCH 2/6] fwk: Deprecate fwk_put_event_and_wait API for single-threaded firmware This patch deprecates the fwk_put_event_and_wait API for builds with BUILD_HAS_MULTITHREADING=no. As this is a synchronous API it could have serious adverse effects on performance and throughput when used in single-threaded mode. Therefore, this API should not be used in the future in single-threaded mode. All future development should follow the deferred response architecture model. Change-Id: I6eccdf4fd901509df14a57a7f6246790061f8409 Signed-off-by: Jim Quigley --- framework/include/fwk_multi_thread.h | 14 +++++++++++++- tools/build_system/rules.mk | 1 + tools/build_system/test.mk | 1 + 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/framework/include/fwk_multi_thread.h b/framework/include/fwk_multi_thread.h index 68c0b0274..133c0e4e5 100644 --- a/framework/include/fwk_multi_thread.h +++ b/framework/include/fwk_multi_thread.h @@ -66,6 +66,10 @@ int fwk_thread_create(fwk_id_t id); * The event identifier and target identifier are validated and must * belong to the same module. * + * Warning: As this API could have serious adverse effects on system + * performance and throughput, this API has been deprecated + * and should not be used in single-threaded mode. + * * \param event Event to put into the queue for processing. Must not be \c NULL. * \param[out] resp_event The response event. Must not be \c NULL. * @@ -77,8 +81,16 @@ int fwk_thread_create(fwk_id_t id); * \retval FWK_E_ACCESS The API is called from an ISR, called from the common * thread, or the event targets the calling thread. */ +#ifdef BUILD_HAS_MULTITHREADING +int fwk_thread_put_event_and_wait(struct fwk_event *event, + struct fwk_event *resp_event); + +#else int fwk_thread_put_event_and_wait(struct fwk_event *event, - struct fwk_event *resp_event); + struct fwk_event *resp_event) + __attribute__((deprecated)); + +#endif /*! * @} diff --git a/tools/build_system/rules.mk b/tools/build_system/rules.mk index fb883d115..016edf169 100644 --- a/tools/build_system/rules.mk +++ b/tools/build_system/rules.mk @@ -99,6 +99,7 @@ endif # set of warnings, and any warnings that do occur are upgraded to errors to # prevent the firmware from building. CFLAGS += -Werror +CFLAGS += -Wno-error=deprecated-declarations CFLAGS += -Wall CFLAGS += -Wextra CFLAGS += -pedantic diff --git a/tools/build_system/test.mk b/tools/build_system/test.mk index 7b83a47fa..115afa3e3 100644 --- a/tools/build_system/test.mk +++ b/tools/build_system/test.mk @@ -21,6 +21,7 @@ CFLAGS += -Wall CFLAGS += -Wextra CFLAGS += -Werror CFLAGS += -Wno-missing-field-initializers +CFLAGS += -Wno-error=deprecated-declarations CFLAGS += -Wno-unused-parameter CFLAGS += -Wno-strict-aliasing CFLAGS += -std=c11 -- GitLab From 3e3a9eb1895d4094d4e459a0d7c633647ce76090 Mon Sep 17 00:00:00 2001 From: Jim Quigley Date: Wed, 6 May 2020 12:48:13 +0100 Subject: [PATCH 3/6] framework/event: Optimize the use of fwk_event in single thread The thread `is_thread_wakeup_event` is never used in single threaded mode. Change-Id: Ic1c54707e34d3ee3a9d84becb8f6bd712b927275 Signed-off-by: Jim Quigley --- framework/include/fwk_event.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/framework/include/fwk_event.h b/framework/include/fwk_event.h index dbbd0a03b..1a69ac198 100644 --- a/framework/include/fwk_event.h +++ b/framework/include/fwk_event.h @@ -76,12 +76,14 @@ struct fwk_event { */ bool is_delayed_response; +#ifdef BUILD_HAS_MULTITHREADING /*! * \internal * \brief Flag indicating whether the event is a response event that a * thread is waiting for to resume execution. */ bool is_thread_wakeup_event; +#endif /*! * \brief Event identifier. -- GitLab From bcd68aad4d658ea22775f3c97ad622d753005c26 Mon Sep 17 00:00:00 2001 From: Jim Quigley Date: Tue, 12 May 2020 15:29:55 +0100 Subject: [PATCH 4/6] fwk/thread: Single-threaded mode for blocking events This patch implements fwk_thread_put_event_and_wait() for platforms which do not support multithreading. Change-Id: Id3e3dd5054b7df378610b2d511de4e08282ac3a4 Signed-off-by: Jim Quigley --- .../include/internal/fwk_single_thread.h | 24 +++ framework/src/fwk_thread.c | 203 +++++++++++++++++- 2 files changed, 220 insertions(+), 7 deletions(-) diff --git a/framework/include/internal/fwk_single_thread.h b/framework/include/internal/fwk_single_thread.h index 2639e0ef3..0cfb92fb9 100644 --- a/framework/include/internal/fwk_single_thread.h +++ b/framework/include/internal/fwk_single_thread.h @@ -40,6 +40,30 @@ struct __fwk_thread_ctx { /* The event currently being processed */ struct fwk_event *current_event; + + /* Event being processed when put_event_and_wait was called */ + struct fwk_event *previous_event; + + /* + * Flag indicating the thread is waiting for the completion of the + * processing of an event (true) or not (false). The thread + * enters this waiting state when calling the fwk_thread_put_and_wait() + * framework API requesting that the execution does not resume at the + * caller until this event processing is complete and the response + * received. The thread leaves this waiting state when the processing + * of the aforementioned event is completed. The execution of the + * caller is then resumed immediately: no other event processing occurs + * between the end of the event processing and the caller execution being + * resumed. + * + * Note that nested put_event_and_wait calls are not supported. + */ + bool waiting_event_processing_completion; + + /* + * The cookie of the event we are waiting for + */ + uint32_t cookie; }; /* diff --git a/framework/src/fwk_thread.c b/framework/src/fwk_thread.c index b00261951..d7ff129f9 100644 --- a/framework/src/fwk_thread.c +++ b/framework/src/fwk_thread.c @@ -34,6 +34,12 @@ static struct __fwk_thread_ctx ctx; static const char err_msg_line[] = "[FWK] Error %d in %s @%d"; static const char err_msg_func[] = "[FWK] Error %d in %s"; +/* States for put_event_and_wait */ +enum wait_states { + WAITING_FOR_EVENT = 0, + WAITING_FOR_RESPONSE = 1, +}; + /* * Static functions */ @@ -74,6 +80,7 @@ static int put_event(struct fwk_event *event) { struct fwk_event *allocated_event; unsigned int interrupt; + bool is_wakeup_event = false; if (event->is_delayed_response) { allocated_event = __fwk_thread_search_delayed_response( @@ -91,6 +98,11 @@ static int put_event(struct fwk_event *event) allocated_event->params, event->params, sizeof(allocated_event->params)); + + /* Is this the event put_event_and_wait is waiting for ? */ + if (ctx.waiting_event_processing_completion && + (ctx.cookie == event->cookie)) + is_wakeup_event = true; } else { allocated_event = duplicate_event(event); if (allocated_event == NULL) @@ -99,6 +111,9 @@ static int put_event(struct fwk_event *event) allocated_event->cookie = event->cookie = ctx.event_cookie_counter++; + if (is_wakeup_event) + ctx.cookie = event->cookie; + if (fwk_interrupt_get_current(&interrupt) != FWK_SUCCESS) fwk_list_push_tail(&ctx.event_queue, &allocated_event->slist_node); else @@ -107,6 +122,13 @@ static int put_event(struct fwk_event *event) return FWK_SUCCESS; } +static void free_event(struct fwk_event *event) +{ + fwk_interrupt_global_disable(); + fwk_list_push_tail(&ctx.free_event_queue, &event->slist_node); + fwk_interrupt_global_enable(); +} + static void process_next_event(void) { int status; @@ -154,16 +176,17 @@ static void process_next_event(void) } } else { status = process_event(event, &async_response_event); - if (status != FWK_SUCCESS) - FWK_LOG_CRIT(err_msg_line, status, __func__, __LINE__); + if ((status != FWK_SUCCESS) && (status != FWK_PENDING)) { + FWK_LOG_CRIT( + "[FWK] Process event (%s: %s -> %s) (%d)\n", + FWK_ID_STR(event->id), + FWK_ID_STR(event->source_id), + FWK_ID_STR(event->target_id), status); + } } ctx.current_event = NULL; - - fwk_interrupt_global_disable(); - fwk_list_push_tail(&ctx.free_event_queue, &event->slist_node); - fwk_interrupt_global_enable(); - + free_event(event); return; } @@ -293,3 +316,169 @@ error: FWK_LOG_CRIT(err_msg_func, status, __func__); return status; } + +int fwk_thread_put_event_and_wait(struct fwk_event *event, + struct fwk_event *resp_event) +{ + const struct fwk_module *module; + int (*process_event)(const struct fwk_event *event, + struct fwk_event *resp_event); + struct fwk_event response_event; + struct fwk_event *next_event; + struct fwk_event *allocated_event; + unsigned int interrupt; + int status = FWK_E_PARAM; + enum wait_states wait_state = WAITING_FOR_EVENT; + + if (!ctx.initialized) { + status = FWK_E_INIT; + goto error; + } + + if ((event == NULL) || (resp_event == NULL)) + goto error; + + if (!fwk_module_is_valid_event_id(event->id)) + goto error; + + if (fwk_interrupt_get_current(&interrupt) == FWK_SUCCESS) { + status = FWK_E_STATE; + goto error; + } + + if (ctx.current_event != NULL) + event->source_id = ctx.current_event->target_id; + else if (!fwk_module_is_valid_entity_id(event->source_id)) { + FWK_LOG_ERR( + "[FWK] deprecated put_event_and_wait (%s: %s -> %s)\n", + FWK_ID_STR(event->id), + FWK_ID_STR(event->source_id), + FWK_ID_STR(event->target_id)); + goto error; + } + + /* No support for nested put_event_and_wait calls */ + if (ctx.waiting_event_processing_completion) { + status = FWK_E_BUSY; + goto error; + } + ctx.waiting_event_processing_completion = true; + ctx.previous_event = ctx.current_event; + + FWK_LOG_TRACE( + "[FWK] deprecated put_event_and_wait (%s: %s -> %s)\n", + FWK_ID_STR(event->id), + FWK_ID_STR(event->source_id), + FWK_ID_STR(event->target_id)); + + event->is_response = false; + event->is_delayed_response = false; + event->response_requested = true; + event->is_notification = false; + + status = put_event(event); + if (status != FWK_SUCCESS) + goto exit; + + ctx.cookie = event->cookie; + + for (;;) { + + if (fwk_list_is_empty(&ctx.event_queue)) { + if (!fwk_list_is_empty(&ctx.isr_event_queue)) + process_isr(); + continue; + } + + ctx.current_event = next_event = FWK_LIST_GET(fwk_list_head( + &ctx.event_queue), struct fwk_event, slist_node); + + if (next_event->cookie != ctx.cookie) { + /* + * Process any events waiting on the event_queue until + * we get to the event from the waiting call. + */ + process_next_event(); + if (!fwk_list_is_empty(&ctx.isr_event_queue)) + process_isr(); + continue; + } + + /* This is either the original event or the response event */ + next_event = FWK_LIST_GET( + fwk_list_pop_head(&ctx.event_queue), struct fwk_event, slist_node); + + if (wait_state == WAITING_FOR_EVENT) { + module = __fwk_module_get_ctx(next_event->target_id)->desc; + process_event = module->process_event; + + response_event = *next_event; + response_event.source_id = next_event->target_id; + response_event.target_id = next_event->source_id; + response_event.is_delayed_response = false; + + /* Execute the event handler */ + status = process_event(next_event, &response_event); + if (status != FWK_SUCCESS) + goto exit; + + /* + * The response event goes onto the queue now + * and we update the cookie to wait for the + * response. + */ + response_event.is_response = true; + response_event.response_requested = false; + if (!response_event.is_delayed_response) { + status = put_event(&response_event); + if (status != FWK_SUCCESS) + goto exit; + ctx.cookie = response_event.cookie; + } else { + allocated_event = duplicate_event(&response_event); + if (allocated_event != NULL) { + fwk_list_push_head( + __fwk_thread_get_delayed_response_list( + response_event.source_id), + &allocated_event->slist_node); + } else { + status = FWK_E_NOMEM; + goto exit; + } + ctx.cookie = allocated_event->cookie; + } + + wait_state = WAITING_FOR_RESPONSE; + free_event(next_event); + + /* + * Check for any interrupt events that might have been + * queued while the event was being executed. + */ + while (!fwk_list_is_empty(&ctx.isr_event_queue)) + process_isr(); + continue; + } + + if (wait_state == WAITING_FOR_RESPONSE) { + /* + * The response event has been received, return to + * the caller. + */ + memcpy(resp_event->params, next_event->params, + sizeof(resp_event->params)); + free_event(next_event); + status = FWK_SUCCESS; + goto exit; + } + } + +exit: + ctx.current_event = ctx.previous_event; + ctx.waiting_event_processing_completion = false; + if (status == FWK_SUCCESS) + return status; +error: + FWK_LOG_CRIT(err_msg_func, status, __func__); + return status; +} -- GitLab From f35bfb3f073d09f3a776e6e3ad76cbf409842c24 Mon Sep 17 00:00:00 2001 From: Jim Quigley Date: Mon, 11 May 2020 10:39:35 +0100 Subject: [PATCH 5/6] juno_debug: Juno debug module must allow single-threaded operation This patch removes the thread creation from the debug module. All operations which could potentially block have been modified to support the deferred response architecture. Change-Id: I85fd86f1a28f92ebaa2e96fad4af930f6182022d Signed-off-by: Jim Quigley --- product/juno/module/juno_debug/src/mod_juno_debug.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/product/juno/module/juno_debug/src/mod_juno_debug.c b/product/juno/module/juno_debug/src/mod_juno_debug.c index 17330b179..58d92e534 100644 --- a/product/juno/module/juno_debug/src/mod_juno_debug.c +++ b/product/juno/module/juno_debug/src/mod_juno_debug.c @@ -407,7 +407,11 @@ static int juno_debug_element_init(fwk_id_t element_id, unsigned int sub_element_count, const void *data) { +#ifdef BUILD_HAS_MULTITHREADING return fwk_thread_create(element_id); +#else + return FWK_SUCCESS; +#endif } static int juno_debug_bind(fwk_id_t id, unsigned int round) -- GitLab From 74bb105d93a2e37539f2feb2a55fb17178e4fab7 Mon Sep 17 00:00:00 2001 From: Jim Quigley Date: Thu, 7 May 2020 17:23:16 +0100 Subject: [PATCH 6/6] juno: Support BUILD_HAS_MULTITHREADING=no This patch allows multi-threading to be disabled for Juno platform. Change-Id: I311bc43f8fb5b57892552f1226527de3ee113715 Signed-off-by: Jim Quigley --- product/juno/scp_ramfw/firmware.mk | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/product/juno/scp_ramfw/firmware.mk b/product/juno/scp_ramfw/firmware.mk index 8ee542da8..ddf46ddf0 100644 --- a/product/juno/scp_ramfw/firmware.mk +++ b/product/juno/scp_ramfw/firmware.mk @@ -56,7 +56,6 @@ BS_FIRMWARE_MODULES := \ juno_thermal BS_FIRMWARE_SOURCES := \ - rtx_config.c \ juno_utils.c \ config_sds.c \ config_log.c \ @@ -92,4 +91,8 @@ BS_FIRMWARE_SOURCES := \ config_juno_thermal.c \ config_scmi_power_domain.c +ifeq ($(BS_FIRMWARE_HAS_MULTITHREADING),yes) + BS_FIRMWARE_SOURCES += rtx_config.c +endif + include $(BS_DIR)/firmware.mk -- GitLab