From b1485d59209bbda536f7f062eef4b63c0ceda1cc Mon Sep 17 00:00:00 2001 From: caveh jalali Date: Fri, 7 Aug 2020 07:24:23 +0000 Subject: Revert "task: Fix mutex_lock() assert (reland)" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 8d46141f4d45c65712a9ca7509b7b60128fa4d89. Reason for revert: getting EC boot loop on volteer: (note that you have to flash EC-RO to get this) 20-08-07 00:22:33.520 --- UART initialized after reboot --- 20-08-07 00:22:33.531 [Image: RO, volteer_1.1.9999-4284ce1 @caveh] 20-08-07 00:22:33.531 [Reset cause: reset-pin] 20-08-07 00:22:33.531 [0.005149 KB boot key mask 0] 20-08-07 00:22:33.543 [0.005438 init buttons] 20-08-07 00:22:33.543 [0.005669 VB Main] 20-08-07 00:22:33.543 [0.005872 VB Ping Cr50] 20-08-07 00:22:33.543 [0.007148 hash start 0x00040000 0x0002f61c] 20-08-07 00:22:33.833 [0.300169 hash done e4ddc3d0ffd015db085389d94faa38d3922e42290b6887baa8de3067ce846c13] 20-08-07 00:22:33.833 [0.300289 VB Verifying hash] 20-08-07 00:22:33.833 ��������������������������������EC\0 \0 �������S��O�8Ӓ.B) h����0g΄l[0.317577 VB Received 0xec00] 20-08-07 00:22:33.850 [0.317899 Jumping to image RW] 20-08-07 00:22:33.850 20-08-07 00:22:33.850 ASSERTION FAILURE '!in_interrupt_context() && task_start_called()' in mutex_lock() at core/cortex-m/task.c:868 20-08-07 00:22:33.861 20-08-07 00:22:33.861 === HANDLER EXCEPTION: 00 ====== xPSR: 0000000a === 20-08-07 00:22:33.861 r0 :00000364 r1 :100b6815 r2 :100b72ab r3 :100956bd 20-08-07 00:22:33.873 r4 :dead6663 r5 :00000364 r6 :200c1c20 r7 :00000001 20-08-07 00:22:33.873 r8 :00001388 r9 :100b4108 r10:100b4158 r11:00000013 20-08-07 00:22:33.884 r12:10095811 sp :200c0320 lr :200c1c20 pc :200c14f8 20-08-07 00:22:33.884 20-08-07 00:22:33.884 cfsr = 0, shcsr = 70000, hfsr = 0, dfsr = 0 20-08-07 00:22:33.884 20-08-07 00:22:33.884 =========== Process Stack Contents =========== 20-08-07 00:22:33.889 00000000: 100cfc00 00002a3d 00002751 00002731 20-08-07 00:22:33.901 00000010: 00002741 00002711 000027e1 00002791 20-08-07 00:22:33.901 00000020: 000027a1 000027b1 00002771 000027c1 20-08-07 00:22:33.901 00000030: 00002721 00002781 00002761 000027d1 20-08-07 00:22:33.906 20-08-07 00:22:33.906 Rebooting... 20-08-07 00:22:33.996 20-08-07 00:22:33.996 20-08-07 00:22:33.996 --- UART initialized after reboot --- Original change's description: > task: Fix mutex_lock() assert (reland) > > mutex_lock() must not be used in interrupt context. Add an assert > to catch this. > > Also assert task_start_called() since task ID is not valid > before this. > > Also remove an old assert since comparing id with TASK_ID_INVALID > doesn't make sense. > > This was first submitted as CL:2309057, then reverted by CL:2323704 > because it broke jump to RW (b/162302011). Fix this by adding check > for task_start_called() to chip/npcx/flash.c and common/i2c_master.c. > > BUG=b:160975910 > BRANCH=none > TEST=boot AP, jump to RW > > Signed-off-by: Edward Hill > Change-Id: I070a265a95d2128643b536814e608509d81adbe3 > Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2335738 > Reviewed-by: Raul E Rangel > Reviewed-by: Denis Brockus Bug: b:160975910 Change-Id: I9e37b1eac7344cddbd756fb45b130d7e0aee661b Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2341706 Reviewed-by: caveh jalali Commit-Queue: caveh jalali Tested-by: caveh jalali --- chip/npcx/flash.c | 11 ++--------- common/i2c_master.c | 4 ---- core/cortex-m/task.c | 8 +------- 3 files changed, 3 insertions(+), 20 deletions(-) diff --git a/chip/npcx/flash.c b/chip/npcx/flash.c index b2b51d15df..5bb8d209b4 100644 --- a/chip/npcx/flash.c +++ b/chip/npcx/flash.c @@ -60,11 +60,8 @@ static void flash_pinmux(int enable) static void flash_execute_cmd(uint8_t code, uint8_t cts) { - /* - * Flash mutex must be held while executing UMA commands after - * task_start(). - */ - ASSERT(!task_start_called() || flash_lock.lock); + /* Flash mutex must be held while executing UMA commands. */ + ASSERT(flash_lock.lock); /* set UMA_CODE */ NPCX_UMA_CODE = code; @@ -726,10 +723,6 @@ int flash_pre_init(void) void flash_lock_mapped_storage(int lock) { - /* Must not call mutex_lock() before task_start(). */ - if (!task_start_called()) - return; - if (lock) mutex_lock(&flash_lock); else diff --git a/common/i2c_master.c b/common/i2c_master.c index 78a5e505b9..f078e690f7 100644 --- a/common/i2c_master.c +++ b/common/i2c_master.c @@ -242,10 +242,6 @@ void i2c_prepare_sysjump(void) { int i; - /* Must not call mutex_lock() before task_start(). */ - if (!task_start_called()) - return; - /* Lock all i2c controllers */ for (i = 0; i < ARRAY_SIZE(port_mutex); ++i) mutex_lock(port_mutex + i); diff --git a/core/cortex-m/task.c b/core/cortex-m/task.c index 61697d2684..75fbf99155 100644 --- a/core/cortex-m/task.c +++ b/core/cortex-m/task.c @@ -860,13 +860,7 @@ void mutex_lock(struct mutex *mtx) uint32_t value; uint32_t id = 1 << task_get_current(); - /* - * mutex_lock() must not be used in interrupt context (because we wait - * if there is contention). Task ID is not valid before task_start() - * (since current_task is scratchpad). - */ - ASSERT(!in_interrupt_context() && task_start_called()); - + ASSERT(id != TASK_ID_INVALID); atomic_or(&mtx->waiters, id); do { -- cgit v1.2.1