From 8e8cf301c25c0f1b846b65bd3108cbab46667f31 Mon Sep 17 00:00:00 2001 From: Gwendal Grignou Date: Thu, 7 Jan 2021 11:44:27 -0800 Subject: common: motion: fix races at shutdown Do not use collection_rate blindly after a function may have slept: the HOOK task could have run suspend() or suspend() call and set it to 0. Fixes 104f5257 ("motion: Control on which task sensor setting functions are running on") [CL:2553347] Conflicts: common/motion_sense.c: Remove indentation that must have happen at merge. BUG=b:176918310, b:170703322, b:251069579 BRANCH=kukui TEST=unit test. Change-Id: I9ef13ceca195db4b48866f1e53f9408fb2bbf595 Signed-off-by: Gwendal Grignou Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2616137 Reviewed-by: Aseda Aboagye Reviewed-by: Diana Z (cherry picked from commit ea99e40f31445e67659c09b32cab6857cad8b83e) Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3960781 Reviewed-by: Eric Yilun Lin Commit-Queue: Eric Yilun Lin Tested-by: Eric Yilun Lin --- common/motion_sense.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/common/motion_sense.c b/common/motion_sense.c index f39ba4e6db..3f9e3c5000 100644 --- a/common/motion_sense.c +++ b/common/motion_sense.c @@ -603,11 +603,8 @@ static inline void update_sense_data(uint8_t *lpc_status, int *psample_id) static int motion_sense_read(struct motion_sensor_t *sensor) { - if (sensor->state != SENSOR_INITIALIZED) - return EC_ERROR_UNKNOWN; - - if (sensor->drv->get_data_rate(sensor) == 0) - return EC_ERROR_NOT_POWERED; + ASSERT(sensor->state == SENSOR_INITIALIZED); + ASSERT(sensor->drv->get_data_rate(sensor) != 0); #ifdef CONFIG_ACCEL_SPOOF_MODE /* @@ -704,8 +701,14 @@ static int motion_sense_process(struct motion_sensor_t *sensor, #endif if (motion_sensor_in_forced_mode(sensor)) { if (motion_sensor_time_to_read(ts, sensor)) { - ret = motion_sense_read(sensor); + /* + * Since motion_sense_read can sleep, other task may be + * scheduled. In particular if suspend is called by + * HOOKS task, it may set colleciton_rate to 0 and we + * would crash in increment_sensor_collection. + */ increment_sensor_collection(sensor, ts); + ret = motion_sense_read(sensor); } else { ret = EC_ERROR_BUSY; } -- cgit v1.2.1