From fb33930b46667a52b3065fa32c659658fe5a5a38 Mon Sep 17 00:00:00 2001 From: Katherine Vincent Date: Tue, 8 Apr 2025 20:45:17 +0100 Subject: [PATCH 1/3] mod/power_domain: Correct Level State Issue The resulting level state from a composite state was being possibly erroneously decremented. This has been addressed in this patch to only decrement if not already zero. Signed-off-by: Katherine Vincent --- module/power_domain/src/power_domain_state_checks.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/module/power_domain/src/power_domain_state_checks.c b/module/power_domain/src/power_domain_state_checks.c index 755039402..cce72a520 100644 --- a/module/power_domain/src/power_domain_state_checks.c +++ b/module/power_domain/src/power_domain_state_checks.c @@ -1,6 +1,6 @@ /* * Arm SCP/MCP Software - * Copyright (c) 2023-2024, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2023-2025, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause * @@ -178,7 +178,9 @@ int get_highest_level_from_composite_state( break; } } - level--; + if (level > 0) { + level--; + } } return (int)level; -- GitLab From 2357963c7fcb479659781f6b5d5c659c4fbb9e23 Mon Sep 17 00:00:00 2001 From: Katherine Vincent Date: Wed, 9 Apr 2025 07:51:09 +0100 Subject: [PATCH 2/3] mod/power_domain: Add Unit Tests Include unit tests to verify the correct of the decrementing state level from composite state. Signed-off-by: Katherine Vincent --- .../mod_power_domain_state_checks_unit_test.c | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/module/power_domain/test/mod_power_domain_state_checks_unit_test.c b/module/power_domain/test/mod_power_domain_state_checks_unit_test.c index b47d86a7f..a5841f329 100644 --- a/module/power_domain/test/mod_power_domain_state_checks_unit_test.c +++ b/module/power_domain/test/mod_power_domain_state_checks_unit_test.c @@ -1,6 +1,6 @@ /* * Arm SCP/MCP Software - * Copyright (c) 2024, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2024-2025, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -371,6 +371,29 @@ void test_get_highest_level_from_comp_state_no_cs_support(void) TEST_ASSERT_EQUAL(0, level); } +void test_get_highest_level_from_comp_state_zero(void) +{ + struct pd_ctx *pd = &mod_pd_ctx_temp.pd_ctx_table[PD_IDX_CLUS0CORE0]; + uint32_t composite_state; + int level; + + pd->cs_support = true; + pd->composite_state_levels_mask = 0; + pd->composite_state_mask_table = core_composite_state_mask_table_UT; + pd->composite_state_mask_table_size = 1; + + /* Force the valid_state_mask to NOT include the first state's bit */ + pd->valid_state_mask = MOD_PD_STATE_ON_MASK; + + /* Ensure there is a composite state with an invalid state at level 0 */ + composite_state = + MOD_PD_COMPOSITE_STATE(MOD_PD_LEVEL_0, 0, 0, 0, MOD_PD_STATE_OFF); + + level = get_highest_level_from_composite_state(pd, composite_state); + + TEST_ASSERT_EQUAL(0, level); +} + void test_is_valid_comp_state_true(void) { struct pd_ctx *pd = &mod_pd_ctx_temp.pd_ctx_table[PD_IDX_CLUS0CORE0]; @@ -606,6 +629,7 @@ int power_domain_state_checks_test_main(void) RUN_TEST(test_get_level_state_from_comp_state_off); RUN_TEST(test_get_highest_level_from_comp_state); RUN_TEST(test_get_highest_level_from_comp_state_no_cs_support); + RUN_TEST(test_get_highest_level_from_comp_state_zero); RUN_TEST(test_is_valid_comp_state_true); RUN_TEST(test_is_valid_comp_state_no_cs_support); RUN_TEST(test_is_valid_comp_state_too_high_level); -- GitLab From 8ca8a110d76b267ecf6c19d52a40d1db4ebd2cb4 Mon Sep 17 00:00:00 2001 From: Katherine Vincent Date: Thu, 10 Apr 2025 15:35:10 +0100 Subject: [PATCH 3/3] mod/power_domain: Remove Casting Inconsistencies with Level States The resulting highest level from a composite state was being cast back and forth from an int to an unsigned int. This patch removes that and maintains consistency as an unsigned int. Signed-off-by: Katherine Vincent --- .../power_domain/include/internal/power_domain.h | 4 ++-- module/power_domain/src/mod_power_domain.c | 4 ++-- .../power_domain/src/power_domain_state_checks.c | 6 +++--- .../test/mocks/Mockmod_power_domain_extra.c | 16 ++++++++-------- .../test/mocks/Mockmod_power_domain_extra.h | 10 +++++----- .../power_domain/test/mod_power_domain_extra.h | 4 ++-- .../mod_power_domain_state_checks_unit_test.c | 6 +++--- 7 files changed, 25 insertions(+), 25 deletions(-) diff --git a/module/power_domain/include/internal/power_domain.h b/module/power_domain/include/internal/power_domain.h index e0ebe41f0..567108397 100644 --- a/module/power_domain/include/internal/power_domain.h +++ b/module/power_domain/include/internal/power_domain.h @@ -1,6 +1,6 @@ /* * Arm SCP/MCP Software - * Copyright (c) 2023-2024, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2023-2025, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause * @@ -415,7 +415,7 @@ unsigned int get_level_state_from_composite_state( * * \return The highest level. */ -int get_highest_level_from_composite_state( +unsigned int get_highest_level_from_composite_state( const struct pd_ctx *pd, uint32_t composite_state); diff --git a/module/power_domain/src/mod_power_domain.c b/module/power_domain/src/mod_power_domain.c index b94c302e6..8761b9372 100644 --- a/module/power_domain/src/mod_power_domain.c +++ b/module/power_domain/src/mod_power_domain.c @@ -1,6 +1,6 @@ /* * Arm SCP/MCP Software - * Copyright (c) 2015-2024, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2015-2025, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause * @@ -231,7 +231,7 @@ static void process_set_state_request( * than the highest power level. */ lowest_level = 0; - highest_level = (unsigned int)get_highest_level_from_composite_state( + highest_level = get_highest_level_from_composite_state( lowest_pd, composite_state); nb_pds = highest_level + 1U; diff --git a/module/power_domain/src/power_domain_state_checks.c b/module/power_domain/src/power_domain_state_checks.c index cce72a520..f57e2c437 100644 --- a/module/power_domain/src/power_domain_state_checks.c +++ b/module/power_domain/src/power_domain_state_checks.c @@ -150,7 +150,7 @@ unsigned int get_level_state_from_composite_state( return (composite_state & mask) >> shift; } -int get_highest_level_from_composite_state( +unsigned int get_highest_level_from_composite_state( const struct pd_ctx *pd, uint32_t composite_state) { @@ -183,7 +183,7 @@ int get_highest_level_from_composite_state( } } - return (int)level; + return level; } bool is_valid_composite_state( @@ -249,7 +249,7 @@ bool is_upwards_transition_propagation( const struct pd_ctx *lowest_pd, uint32_t composite_state) { - int highest_level, level; + unsigned int highest_level, level; const struct pd_ctx *pd; unsigned int state; const uint32_t *state_mask_table; diff --git a/module/power_domain/test/mocks/Mockmod_power_domain_extra.c b/module/power_domain/test/mocks/Mockmod_power_domain_extra.c index a8342439c..1585e8eba 100644 --- a/module/power_domain/test/mocks/Mockmod_power_domain_extra.c +++ b/module/power_domain/test/mocks/Mockmod_power_domain_extra.c @@ -252,7 +252,7 @@ typedef struct _CMOCK_get_highest_level_from_composite_state_CALL_INSTANCE { UNITY_LINE_TYPE LineNumber; char ExpectAnyArgsBool; - int ReturnVal; + unsigned int ReturnVal; const struct pd_ctx* Expected_pd; uint32_t Expected_composite_state; int Expected_pd_Depth; @@ -470,7 +470,7 @@ static struct Mockmod_power_domain_extraInstance int get_level_state_from_composite_state_CallbackCalls; CMOCK_MEM_INDEX_TYPE get_level_state_from_composite_state_CallInstance; char get_highest_level_from_composite_state_IgnoreBool; - int get_highest_level_from_composite_state_FinalReturn; + unsigned int get_highest_level_from_composite_state_FinalReturn; char get_highest_level_from_composite_state_CallbackBool; CMOCK_get_highest_level_from_composite_state_CALLBACK get_highest_level_from_composite_state_CallbackFunctionPointer; int get_highest_level_from_composite_state_CallbackCalls; @@ -3102,7 +3102,7 @@ void get_level_state_from_composite_state_CMockIgnoreArg_level(UNITY_LINE_TYPE c cmock_call_instance->IgnoreArg_level = 1; } -int get_highest_level_from_composite_state(const struct pd_ctx* pd, uint32_t composite_state) +unsigned int get_highest_level_from_composite_state(const struct pd_ctx* pd, uint32_t composite_state) { UNITY_LINE_TYPE cmock_line = TEST_LINE_NUM; CMOCK_get_highest_level_from_composite_state_CALL_INSTANCE* cmock_call_instance; @@ -3120,7 +3120,7 @@ int get_highest_level_from_composite_state(const struct pd_ctx* pd, uint32_t com if (!Mock.get_highest_level_from_composite_state_CallbackBool && Mock.get_highest_level_from_composite_state_CallbackFunctionPointer != NULL) { - int cmock_cb_ret = Mock.get_highest_level_from_composite_state_CallbackFunctionPointer(pd, composite_state, Mock.get_highest_level_from_composite_state_CallbackCalls++); + unsigned int cmock_cb_ret = Mock.get_highest_level_from_composite_state_CallbackFunctionPointer(pd, composite_state, Mock.get_highest_level_from_composite_state_CallbackCalls++); UNITY_CLR_DETAILS(); return cmock_cb_ret; } @@ -3160,7 +3160,7 @@ void CMockExpectParameters_get_highest_level_from_composite_state(CMOCK_get_high cmock_call_instance->IgnoreArg_composite_state = 0; } -void get_highest_level_from_composite_state_CMockIgnoreAndReturn(UNITY_LINE_TYPE cmock_line, int cmock_to_return) +void get_highest_level_from_composite_state_CMockIgnoreAndReturn(UNITY_LINE_TYPE cmock_line, unsigned int cmock_to_return) { CMOCK_MEM_INDEX_TYPE cmock_guts_index = CMock_Guts_MemNew(sizeof(CMOCK_get_highest_level_from_composite_state_CALL_INSTANCE)); CMOCK_get_highest_level_from_composite_state_CALL_INSTANCE* cmock_call_instance = (CMOCK_get_highest_level_from_composite_state_CALL_INSTANCE*)CMock_Guts_GetAddressFor(cmock_guts_index); @@ -3181,7 +3181,7 @@ void get_highest_level_from_composite_state_CMockStopIgnore(void) Mock.get_highest_level_from_composite_state_IgnoreBool = (char)0; } -void get_highest_level_from_composite_state_CMockExpectAnyArgsAndReturn(UNITY_LINE_TYPE cmock_line, int cmock_to_return) +void get_highest_level_from_composite_state_CMockExpectAnyArgsAndReturn(UNITY_LINE_TYPE cmock_line, unsigned int cmock_to_return) { CMOCK_MEM_INDEX_TYPE cmock_guts_index = CMock_Guts_MemNew(sizeof(CMOCK_get_highest_level_from_composite_state_CALL_INSTANCE)); CMOCK_get_highest_level_from_composite_state_CALL_INSTANCE* cmock_call_instance = (CMOCK_get_highest_level_from_composite_state_CALL_INSTANCE*)CMock_Guts_GetAddressFor(cmock_guts_index); @@ -3195,7 +3195,7 @@ void get_highest_level_from_composite_state_CMockExpectAnyArgsAndReturn(UNITY_LI cmock_call_instance->ExpectAnyArgsBool = (char)1; } -void get_highest_level_from_composite_state_CMockExpectAndReturn(UNITY_LINE_TYPE cmock_line, const struct pd_ctx* pd, uint32_t composite_state, int cmock_to_return) +void get_highest_level_from_composite_state_CMockExpectAndReturn(UNITY_LINE_TYPE cmock_line, const struct pd_ctx* pd, uint32_t composite_state, unsigned int cmock_to_return) { CMOCK_MEM_INDEX_TYPE cmock_guts_index = CMock_Guts_MemNew(sizeof(CMOCK_get_highest_level_from_composite_state_CALL_INSTANCE)); CMOCK_get_highest_level_from_composite_state_CALL_INSTANCE* cmock_call_instance = (CMOCK_get_highest_level_from_composite_state_CALL_INSTANCE*)CMock_Guts_GetAddressFor(cmock_guts_index); @@ -3223,7 +3223,7 @@ void get_highest_level_from_composite_state_Stub(CMOCK_get_highest_level_from_co Mock.get_highest_level_from_composite_state_CallbackFunctionPointer = Callback; } -void get_highest_level_from_composite_state_CMockExpectWithArrayAndReturn(UNITY_LINE_TYPE cmock_line, const struct pd_ctx* pd, int pd_Depth, uint32_t composite_state, int cmock_to_return) +void get_highest_level_from_composite_state_CMockExpectWithArrayAndReturn(UNITY_LINE_TYPE cmock_line, const struct pd_ctx* pd, int pd_Depth, uint32_t composite_state, unsigned int cmock_to_return) { CMOCK_MEM_INDEX_TYPE cmock_guts_index = CMock_Guts_MemNew(sizeof(CMOCK_get_highest_level_from_composite_state_CALL_INSTANCE)); CMOCK_get_highest_level_from_composite_state_CALL_INSTANCE* cmock_call_instance = (CMOCK_get_highest_level_from_composite_state_CALL_INSTANCE*)CMock_Guts_GetAddressFor(cmock_guts_index); diff --git a/module/power_domain/test/mocks/Mockmod_power_domain_extra.h b/module/power_domain/test/mocks/Mockmod_power_domain_extra.h index 62800f0bd..d4c9044d6 100644 --- a/module/power_domain/test/mocks/Mockmod_power_domain_extra.h +++ b/module/power_domain/test/mocks/Mockmod_power_domain_extra.h @@ -299,19 +299,19 @@ void get_level_state_from_composite_state_CMockIgnoreArg_composite_state(UNITY_L #define get_level_state_from_composite_state_IgnoreArg_level() get_level_state_from_composite_state_CMockIgnoreArg_level(__LINE__) void get_level_state_from_composite_state_CMockIgnoreArg_level(UNITY_LINE_TYPE cmock_line); #define get_highest_level_from_composite_state_IgnoreAndReturn(cmock_retval) get_highest_level_from_composite_state_CMockIgnoreAndReturn(__LINE__, cmock_retval) -void get_highest_level_from_composite_state_CMockIgnoreAndReturn(UNITY_LINE_TYPE cmock_line, int cmock_to_return); +void get_highest_level_from_composite_state_CMockIgnoreAndReturn(UNITY_LINE_TYPE cmock_line, unsigned int cmock_to_return); #define get_highest_level_from_composite_state_StopIgnore() get_highest_level_from_composite_state_CMockStopIgnore() void get_highest_level_from_composite_state_CMockStopIgnore(void); #define get_highest_level_from_composite_state_ExpectAnyArgsAndReturn(cmock_retval) get_highest_level_from_composite_state_CMockExpectAnyArgsAndReturn(__LINE__, cmock_retval) -void get_highest_level_from_composite_state_CMockExpectAnyArgsAndReturn(UNITY_LINE_TYPE cmock_line, int cmock_to_return); +void get_highest_level_from_composite_state_CMockExpectAnyArgsAndReturn(UNITY_LINE_TYPE cmock_line, unsigned int cmock_to_return); #define get_highest_level_from_composite_state_ExpectAndReturn(pd, composite_state, cmock_retval) get_highest_level_from_composite_state_CMockExpectAndReturn(__LINE__, pd, composite_state, cmock_retval) -void get_highest_level_from_composite_state_CMockExpectAndReturn(UNITY_LINE_TYPE cmock_line, const struct pd_ctx* pd, uint32_t composite_state, int cmock_to_return); -typedef int (* CMOCK_get_highest_level_from_composite_state_CALLBACK)(const struct pd_ctx* pd, uint32_t composite_state, int cmock_num_calls); +void get_highest_level_from_composite_state_CMockExpectAndReturn(UNITY_LINE_TYPE cmock_line, const struct pd_ctx* pd, uint32_t composite_state, unsigned int cmock_to_return); +typedef unsigned int (* CMOCK_get_highest_level_from_composite_state_CALLBACK)(const struct pd_ctx* pd, uint32_t composite_state, int cmock_num_calls); void get_highest_level_from_composite_state_AddCallback(CMOCK_get_highest_level_from_composite_state_CALLBACK Callback); void get_highest_level_from_composite_state_Stub(CMOCK_get_highest_level_from_composite_state_CALLBACK Callback); #define get_highest_level_from_composite_state_StubWithCallback get_highest_level_from_composite_state_Stub #define get_highest_level_from_composite_state_ExpectWithArrayAndReturn(pd, pd_Depth, composite_state, cmock_retval) get_highest_level_from_composite_state_CMockExpectWithArrayAndReturn(__LINE__, pd, pd_Depth, composite_state, cmock_retval) -void get_highest_level_from_composite_state_CMockExpectWithArrayAndReturn(UNITY_LINE_TYPE cmock_line, const struct pd_ctx* pd, int pd_Depth, uint32_t composite_state, int cmock_to_return); +void get_highest_level_from_composite_state_CMockExpectWithArrayAndReturn(UNITY_LINE_TYPE cmock_line, const struct pd_ctx* pd, int pd_Depth, uint32_t composite_state, unsigned int cmock_to_return); #define get_highest_level_from_composite_state_IgnoreArg_pd() get_highest_level_from_composite_state_CMockIgnoreArg_pd(__LINE__) void get_highest_level_from_composite_state_CMockIgnoreArg_pd(UNITY_LINE_TYPE cmock_line); #define get_highest_level_from_composite_state_IgnoreArg_composite_state() get_highest_level_from_composite_state_CMockIgnoreArg_composite_state(__LINE__) diff --git a/module/power_domain/test/mod_power_domain_extra.h b/module/power_domain/test/mod_power_domain_extra.h index cdef4ef61..0a7ebcea3 100644 --- a/module/power_domain/test/mod_power_domain_extra.h +++ b/module/power_domain/test/mod_power_domain_extra.h @@ -1,6 +1,6 @@ /* * Arm SCP/MCP Software - * Copyright (c) 2023-2024, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2023-2025, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause * @@ -57,7 +57,7 @@ unsigned int get_level_state_from_composite_state( uint32_t composite_state, int level); -int get_highest_level_from_composite_state( +unsigned int get_highest_level_from_composite_state( const struct pd_ctx *pd, uint32_t composite_state); diff --git a/module/power_domain/test/mod_power_domain_state_checks_unit_test.c b/module/power_domain/test/mod_power_domain_state_checks_unit_test.c index a5841f329..44c4a1426 100644 --- a/module/power_domain/test/mod_power_domain_state_checks_unit_test.c +++ b/module/power_domain/test/mod_power_domain_state_checks_unit_test.c @@ -350,7 +350,7 @@ void test_get_highest_level_from_comp_state(void) struct pd_ctx *pd = &mod_pd_ctx_temp.pd_ctx_table[PD_IDX_CLUS0CORE0]; uint32_t composite_state = MOD_PD_COMPOSITE_STATE( MOD_PD_LEVEL_2, 0, MOD_PD_STATE_ON, MOD_PD_STATE_OFF, MOD_PD_STATE_OFF); - int level; + unsigned int level; pd->cs_support = true; level = get_highest_level_from_composite_state(pd, composite_state); @@ -363,7 +363,7 @@ void test_get_highest_level_from_comp_state_no_cs_support(void) struct pd_ctx *pd = &mod_pd_ctx_temp.pd_ctx_table[PD_IDX_CLUS0CORE0]; uint32_t composite_state = MOD_PD_COMPOSITE_STATE( MOD_PD_LEVEL_2, 0, MOD_PD_STATE_ON, MOD_PD_STATE_OFF, MOD_PD_STATE_OFF); - int level; + unsigned int level; pd->cs_support = false; level = get_highest_level_from_composite_state(pd, composite_state); @@ -375,7 +375,7 @@ void test_get_highest_level_from_comp_state_zero(void) { struct pd_ctx *pd = &mod_pd_ctx_temp.pd_ctx_table[PD_IDX_CLUS0CORE0]; uint32_t composite_state; - int level; + unsigned int level; pd->cs_support = true; pd->composite_state_levels_mask = 0; -- GitLab