diff options
author | Wai-Hong Tam <waihong@google.com> | 2021-07-21 15:11:59 -0700 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2021-07-23 22:14:04 +0000 |
commit | e8f6467d65f086a16ec33cb3cfd724086595d2f1 (patch) | |
tree | e633d245112862427696f51e11d35a90a17e0442 | |
parent | 6521a7aa8d69c494e98667d1ff05ec2c2d784287 (diff) | |
download | chrome-ec-e8f6467d65f086a16ec33cb3cfd724086595d2f1.tar.gz |
tablet_mode: Fix the race condition of setting the tablet mode
There are 2 tablet mode triggers:
* Lid: when the lid angle close to 360 degree, either detected by
a GMR sensor or by the angle computed by 2 motion sensors.
* Base: when the detachable base is detached.
These 2 triggers set the tablet mode status, which as a boolean,
resulting a race condition.
This CL fixes the race condition. Each trigger updates its own bit. The
final tablet mode status is the OR of all bits.
BRANCH=Trogdor
BUG=b:193873098
TEST=Attached the base, checked tablet_mode enabled;
detached the base, checked tablet_mode disabled;
flip base to 360 degree, checked tablet_mode disabled;
move the base out of the lid, checked tablet_mode disabled.
Change-Id: Ia9d9d2d66c194796c1039cc8b746c8d1f28a4496
Signed-off-by: Wai-Hong Tam <waihong@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3044414
Reviewed-by: Ting Shen <phoenixshen@chromium.org>
-rw-r--r-- | board/aleena/board.c | 2 | ||||
-rw-r--r-- | board/burnet/board.c | 2 | ||||
-rw-r--r-- | board/cheza/base_detect.c | 2 | ||||
-rw-r--r-- | board/coachz/base_detect.c | 2 | ||||
-rw-r--r-- | board/coral/board.c | 4 | ||||
-rw-r--r-- | board/dalboz/board.c | 2 | ||||
-rw-r--r-- | board/dirinboz/board.c | 2 | ||||
-rw-r--r-- | board/eve/board.c | 2 | ||||
-rw-r--r-- | board/gumboz/board.c | 2 | ||||
-rw-r--r-- | board/homestar/base_detect.c | 2 | ||||
-rw-r--r-- | board/jacuzzi/board.c | 2 | ||||
-rw-r--r-- | board/lindar/board.c | 2 | ||||
-rw-r--r-- | board/lingcod/board.c | 2 | ||||
-rw-r--r-- | board/malefor/board.c | 2 | ||||
-rw-r--r-- | board/mrbland/base_detect.c | 2 | ||||
-rw-r--r-- | board/nuwani/board.c | 2 | ||||
-rw-r--r-- | board/poppy/base_detect_lux.c | 2 | ||||
-rw-r--r-- | board/poppy/base_detect_poppy.c | 2 | ||||
-rw-r--r-- | board/reef/board.c | 3 | ||||
-rw-r--r-- | board/reef_it8320/board.c | 2 | ||||
-rw-r--r-- | board/reef_mchp/board.c | 3 | ||||
-rw-r--r-- | board/shuboz/board.c | 2 | ||||
-rw-r--r-- | board/treeya/board.c | 2 | ||||
-rw-r--r-- | board/vilboz/board.c | 2 | ||||
-rw-r--r-- | common/motion_lid.c | 2 | ||||
-rw-r--r-- | common/motion_sense.c | 2 | ||||
-rw-r--r-- | common/tablet_mode.c | 28 | ||||
-rw-r--r-- | include/tablet_mode.h | 7 | ||||
-rw-r--r-- | test/motion_angle_tablet.c | 4 |
29 files changed, 54 insertions, 41 deletions
diff --git a/board/aleena/board.c b/board/aleena/board.c index ded6cbf698..0343a399c5 100644 --- a/board/aleena/board.c +++ b/board/aleena/board.c @@ -169,7 +169,7 @@ void board_update_sensor_config_from_sku(void) } else { motion_sensor_count = 0; /* Device is clamshell only */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); /* Gyro is not present, don't allow line to float */ gpio_set_flags(GPIO_6AXIS_INT_L, GPIO_INPUT | GPIO_PULL_DOWN); diff --git a/board/burnet/board.c b/board/burnet/board.c index a272702ebe..f0a8ec9824 100644 --- a/board/burnet/board.c +++ b/board/burnet/board.c @@ -537,7 +537,7 @@ static void board_init(void) } else { motion_sensor_count = 0; /* Device is clamshell only */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); /* Turn off GMR interrupt */ gmr_tablet_switch_disable(); /* Base accel is not stuffed, don't allow line to float */ diff --git a/board/cheza/base_detect.c b/board/cheza/base_detect.c index b47504144b..1703588dd0 100644 --- a/board/cheza/base_detect.c +++ b/board/cheza/base_detect.c @@ -101,7 +101,7 @@ static void base_detect_change(enum base_status status) /* We don't enable dual-battery support. Set the base power directly. */ gpio_set_level(GPIO_EN_PPVAR_VAR_BASE, connected); - tablet_set_mode(!connected); + tablet_set_mode(!connected, TABLET_TRIGGER_BASE); } static void print_base_detect_value(const char *str, int v) diff --git a/board/coachz/base_detect.c b/board/coachz/base_detect.c index 1a27538166..39e478d1c7 100644 --- a/board/coachz/base_detect.c +++ b/board/coachz/base_detect.c @@ -83,7 +83,7 @@ static void base_detect_change(enum base_status status) return; gpio_set_level(GPIO_EN_BASE, connected); - tablet_set_mode(!connected); + tablet_set_mode(!connected, TABLET_TRIGGER_BASE); base_set_state(connected); current_base_status = status; } diff --git a/board/coral/board.c b/board/coral/board.c index d79aeef8c6..2a63a60d13 100644 --- a/board/coral/board.c +++ b/board/coral/board.c @@ -456,7 +456,7 @@ static void board_set_tablet_mode(void) if (SKU_IS_CONVERTIBLE(sku_id)) tablet_mode = !gpio_get_level(GPIO_TABLET_MODE_L); - tablet_set_mode(tablet_mode); + tablet_set_mode(tablet_mode, TABLET_TRIGGER_LID); } /* Initialize board. */ @@ -911,7 +911,7 @@ static void sku_id_init(void) CPRINTS("Disable tablet mode interrupt"); gpio_disable_interrupt(GPIO_TABLET_MODE_L); /* Enfore device in laptop mode */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); } } } diff --git a/board/dalboz/board.c b/board/dalboz/board.c index 18d98b29e5..44015110cf 100644 --- a/board/dalboz/board.c +++ b/board/dalboz/board.c @@ -607,7 +607,7 @@ static void setup_fw_config(void) } else { motion_sensor_count = 0; /* Device is clamshell only */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); /* Gyro is not present, don't allow line to float */ gpio_set_flags(GPIO_6AXIS_INT_L, GPIO_INPUT | GPIO_PULL_DOWN); } diff --git a/board/dirinboz/board.c b/board/dirinboz/board.c index 1daf425b2a..d7b3446197 100644 --- a/board/dirinboz/board.c +++ b/board/dirinboz/board.c @@ -493,7 +493,7 @@ static void setup_fw_config(void) } else { motion_sensor_count = 0; /* Device is clamshell only */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); /* Gyro is not present, don't allow line to float */ gpio_set_flags(GPIO_6AXIS_INT_L, GPIO_INPUT | GPIO_PULL_DOWN); } diff --git a/board/eve/board.c b/board/eve/board.c index 31c9004da4..81ea080e26 100644 --- a/board/eve/board.c +++ b/board/eve/board.c @@ -459,7 +459,7 @@ static void board_set_tablet_mode(void) { int flipped_360_mode = !gpio_get_level(GPIO_TABLET_MODE_L); - tablet_set_mode(flipped_360_mode); + tablet_set_mode(flipped_360_mode, TABLET_TRIGGER_LID); /* Update DPTF profile based on mode */ if (flipped_360_mode) diff --git a/board/gumboz/board.c b/board/gumboz/board.c index 8108798d63..b04a09f0f5 100644 --- a/board/gumboz/board.c +++ b/board/gumboz/board.c @@ -497,7 +497,7 @@ static void setup_fw_config(void) } else { motion_sensor_count = 0; /* Device is clamshell only */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); /* Gyro is not present, don't allow line to float */ gpio_set_flags(GPIO_6AXIS_INT_L, GPIO_INPUT | GPIO_PULL_DOWN); } diff --git a/board/homestar/base_detect.c b/board/homestar/base_detect.c index 5527018160..2bd115ea2a 100644 --- a/board/homestar/base_detect.c +++ b/board/homestar/base_detect.c @@ -86,7 +86,7 @@ static void base_detect_change(enum base_status status) return; gpio_set_level(GPIO_EN_BASE, connected); - tablet_set_mode(!connected); + tablet_set_mode(!connected, TABLET_TRIGGER_BASE); base_set_state(connected); current_base_status = status; } diff --git a/board/jacuzzi/board.c b/board/jacuzzi/board.c index dcbd69b726..810f332c50 100644 --- a/board/jacuzzi/board.c +++ b/board/jacuzzi/board.c @@ -341,7 +341,7 @@ static void board_init(void) GPIO_INPUT | GPIO_PULL_DOWN); #endif /* !VARIANT_KUKUI_NO_SENSORS */ /* Disable tablet mode. */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); gmr_tablet_switch_disable(); gpio_set_flags(GPIO_TABLET_MODE_L, GPIO_INPUT | GPIO_PULL_UP); diff --git a/board/lindar/board.c b/board/lindar/board.c index 17d60a7f9b..621d0d305d 100644 --- a/board/lindar/board.c +++ b/board/lindar/board.c @@ -76,7 +76,7 @@ static void board_init(void) } else { motion_sensor_count = 0; /* Device is clamshell only */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); /* Gyro is not present, don't allow line to float */ gpio_set_flags(GPIO_EC_IMU_INT_L, GPIO_INPUT | GPIO_PULL_DOWN); } diff --git a/board/lingcod/board.c b/board/lingcod/board.c index a8151b94e7..88913a63af 100644 --- a/board/lingcod/board.c +++ b/board/lingcod/board.c @@ -75,7 +75,7 @@ static void board_init(void) } else { motion_sensor_count = 0; /* Device is clamshell only */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); /* Gyro is not present, don't allow line to float */ gpio_set_flags(GPIO_EC_IMU_INT_L, GPIO_INPUT | GPIO_PULL_DOWN); } diff --git a/board/malefor/board.c b/board/malefor/board.c index 059a1bf6da..394aec8ee7 100644 --- a/board/malefor/board.c +++ b/board/malefor/board.c @@ -76,7 +76,7 @@ static void board_init(void) } else { motion_sensor_count = 0; /* Device is clamshell only */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); /* Gyro is not present, don't allow line to float */ gpio_set_flags(GPIO_EC_IMU_INT_L, GPIO_INPUT | GPIO_PULL_DOWN); } diff --git a/board/mrbland/base_detect.c b/board/mrbland/base_detect.c index 5527018160..2bd115ea2a 100644 --- a/board/mrbland/base_detect.c +++ b/board/mrbland/base_detect.c @@ -86,7 +86,7 @@ static void base_detect_change(enum base_status status) return; gpio_set_level(GPIO_EN_BASE, connected); - tablet_set_mode(!connected); + tablet_set_mode(!connected, TABLET_TRIGGER_BASE); base_set_state(connected); current_base_status = status; } diff --git a/board/nuwani/board.c b/board/nuwani/board.c index 5d5f5f842d..8004e25b37 100644 --- a/board/nuwani/board.c +++ b/board/nuwani/board.c @@ -172,7 +172,7 @@ void board_update_sensor_config_from_sku(void) } else { motion_sensor_count = 0; /* Device is clamshell only */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); /* Gyro is not present, don't allow line to float */ gpio_set_flags(GPIO_6AXIS_INT_L, GPIO_INPUT | GPIO_PULL_DOWN); diff --git a/board/poppy/base_detect_lux.c b/board/poppy/base_detect_lux.c index 14a937a7fd..37ed3304fe 100644 --- a/board/poppy/base_detect_lux.c +++ b/board/poppy/base_detect_lux.c @@ -128,7 +128,7 @@ static void base_detect_change(enum base_status status) */ task_wake(TASK_ID_CHARGER); - tablet_set_mode(!connected); + tablet_set_mode(!connected, TABLET_TRIGGER_BASE); } static void print_base_detect_value(const char *str, int v) diff --git a/board/poppy/base_detect_poppy.c b/board/poppy/base_detect_poppy.c index 8a202a21d9..9638894cb8 100644 --- a/board/poppy/base_detect_poppy.c +++ b/board/poppy/base_detect_poppy.c @@ -95,7 +95,7 @@ static void base_detect_change(enum base_status status) CPRINTS("Base %sconnected", connected ? "" : "not "); gpio_set_level(GPIO_PP3300_DX_BASE, connected); - tablet_set_mode(!connected); + tablet_set_mode(!connected, TABLET_TRIGGER_BASE); current_base_status = status; if (connected) diff --git a/board/reef/board.c b/board/reef/board.c index 26e70fa52b..26f5b8dceb 100644 --- a/board/reef/board.c +++ b/board/reef/board.c @@ -445,7 +445,8 @@ void chipset_pre_init_callback(void) static void board_set_tablet_mode(void) { - tablet_set_mode(!gpio_get_level(GPIO_TABLET_MODE_L)); + tablet_set_mode(!gpio_get_level(GPIO_TABLET_MODE_L), + TABLET_TRIGGER_LID); } /* Initialize board. */ diff --git a/board/reef_it8320/board.c b/board/reef_it8320/board.c index 43170dd04d..83106698f0 100644 --- a/board/reef_it8320/board.c +++ b/board/reef_it8320/board.c @@ -208,7 +208,7 @@ static void board_set_tablet_mode(void) * Always report device isn't in tablet mode because * our id is clamshell and no TABLET_MODE_L pin */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); } /* Initialize board. */ diff --git a/board/reef_mchp/board.c b/board/reef_mchp/board.c index d8e0d22fac..d5212ca931 100644 --- a/board/reef_mchp/board.c +++ b/board/reef_mchp/board.c @@ -639,7 +639,8 @@ void chipset_pre_init_callback(void) static void board_set_tablet_mode(void) { - tablet_set_mode(!gpio_get_level(GPIO_TABLET_MODE_L)); + tablet_set_mode(!gpio_get_level(GPIO_TABLET_MODE_L), + TABLET_TRIGGER_LID); } /* Initialize board. */ diff --git a/board/shuboz/board.c b/board/shuboz/board.c index e14c69b893..114bae6ac8 100644 --- a/board/shuboz/board.c +++ b/board/shuboz/board.c @@ -585,7 +585,7 @@ static void setup_fw_config(void) } else { motion_sensor_count = 0; /* Device is clamshell only */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); /* Gyro is not present, don't allow line to float */ gpio_set_flags(GPIO_6AXIS_INT_L, GPIO_INPUT | GPIO_PULL_DOWN); } diff --git a/board/treeya/board.c b/board/treeya/board.c index 504d171c33..4583d89d7c 100644 --- a/board/treeya/board.c +++ b/board/treeya/board.c @@ -172,7 +172,7 @@ void board_update_sensor_config_from_sku(void) } else { motion_sensor_count = 0; /* Device is clamshell only */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); /* Gyro is not present, don't allow line to float */ gpio_set_flags(GPIO_6AXIS_INT_L, GPIO_INPUT | GPIO_PULL_DOWN); diff --git a/board/vilboz/board.c b/board/vilboz/board.c index ef0135d374..8351834b78 100644 --- a/board/vilboz/board.c +++ b/board/vilboz/board.c @@ -409,7 +409,7 @@ static void setup_fw_config(void) } else { motion_sensor_count = 0; /* Device is clamshell only */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); /* Gyro is not present, don't allow line to float */ gpio_set_flags(GPIO_6AXIS_INT_L, GPIO_INPUT | GPIO_PULL_DOWN); } diff --git a/common/motion_lid.c b/common/motion_lid.c index 1d84c17eff..eb0297aefa 100644 --- a/common/motion_lid.c +++ b/common/motion_lid.c @@ -176,7 +176,7 @@ static void motion_lid_set_tablet_mode(int reliable) /* Alright, we're convinced. */ tablet_mode_debounce_cnt = TABLET_MODE_DEBOUNCE_COUNT; - tablet_set_mode(new_mode); + tablet_set_mode(new_mode, TABLET_TRIGGER_LID); return; } tablet_mode_debounce_cnt--; diff --git a/common/motion_sense.c b/common/motion_sense.c index 92690d7aea..114f240fba 100644 --- a/common/motion_sense.c +++ b/common/motion_sense.c @@ -416,7 +416,7 @@ static void motion_sense_switch_sensor_rate(void) (ret != EC_SUCCESS) && (i == CONFIG_LID_ANGLE_SENSOR_BASE || i == CONFIG_LID_ANGLE_SENSOR_LID)) - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); } } else { /* The sensors are being powered off */ diff --git a/common/tablet_mode.c b/common/tablet_mode.c index 68d90ae772..3a5616ec3f 100644 --- a/common/tablet_mode.c +++ b/common/tablet_mode.c @@ -17,10 +17,10 @@ #define CPRINTF(format, args...) cprintf(CC_MOTION_LID, format, ## args) /* - * Other code modules assume that notebook mode (i.e. tablet_mode = false) at + * Other code modules assume that notebook mode (i.e. tablet_mode = 0) at * startup */ -static bool tablet_mode; +static uint32_t tablet_mode; /* * Console command can force the value of tablet_mode. If tablet_mode_force is @@ -39,7 +39,7 @@ static bool disabled; int tablet_get_mode(void) { - return tablet_mode; + return !!tablet_mode; } static inline void print_tablet_mode(void) @@ -61,13 +61,21 @@ static void notify_tablet_mode_change(void) } -void tablet_set_mode(int mode) +void tablet_set_mode(int mode, uint32_t trigger) { + uint32_t old_mode = tablet_mode; + /* If tablet_mode is forced via a console command, ignore set. */ if (tablet_mode_forced) return; - if (tablet_mode == !!mode) + if (mode) + tablet_mode |= trigger; + else + tablet_mode &= ~trigger; + + /* Boolean comparison */ + if (!tablet_mode == !old_mode) return; if (disabled) { @@ -81,14 +89,12 @@ void tablet_set_mode(int mode) return; } - tablet_mode = !!mode; - notify_tablet_mode_change(); } void tablet_disable(void) { - tablet_mode = false; + tablet_mode = 0; disabled = true; } @@ -131,7 +137,7 @@ static void gmr_tablet_switch_interrupt_debounce(void) */ if (!IS_ENABLED(CONFIG_LID_ANGLE) || gmr_sensor_at_360) - tablet_set_mode(gmr_sensor_at_360); + tablet_set_mode(gmr_sensor_at_360, TABLET_TRIGGER_LID); if (IS_ENABLED(CONFIG_LID_ANGLE_UPDATE) && gmr_sensor_at_360) lid_angle_peripheral_enable(0); @@ -182,10 +188,10 @@ static int command_settabletmode(int argc, char **argv) return EC_ERROR_PARAM_COUNT; if (argv[1][0] == 'o' && argv[1][1] == 'n') { - tablet_mode = true; + tablet_mode = TABLET_TRIGGER_LID; tablet_mode_forced = true; } else if (argv[1][0] == 'o' && argv[1][1] == 'f') { - tablet_mode = false; + tablet_mode = 0; tablet_mode_forced = true; } else if (argv[1][0] == 'r') { tablet_mode_forced = false; diff --git a/include/tablet_mode.h b/include/tablet_mode.h index b5a2f2f562..3e03d77f6a 100644 --- a/include/tablet_mode.h +++ b/include/tablet_mode.h @@ -13,12 +13,17 @@ */ int tablet_get_mode(void); +/* Bit mask of tablet mode trigger */ +#define TABLET_TRIGGER_LID BIT(0) +#define TABLET_TRIGGER_BASE BIT(1) + /** * Set tablet mode state * * @param mode 1: tablet mode. 0 clamshell mode. + * @param trigger: bitmask of the trigger, TABLET_TRIGGER_*. */ -void tablet_set_mode(int mode); +void tablet_set_mode(int mode, uint32_t trigger); /** * Disable tablet mode diff --git a/test/motion_angle_tablet.c b/test/motion_angle_tablet.c index 3cf496ac5b..8eea053405 100644 --- a/test/motion_angle_tablet.c +++ b/test/motion_angle_tablet.c @@ -57,7 +57,7 @@ static int test_lid_angle_less180(void) cprints(CC_ACCEL, "start loop"); /* Force clamshell mode, to be sure we go in tablet mode ASAP. */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); /* Check we stay in tablet mode, even when hinge is vertical. */ while (index < kAccelerometerVerticalHingeTestDataLength) { @@ -79,7 +79,7 @@ static int test_lid_angle_less180(void) * Check we stay in tablet mode, even when hinge is vertical and * shaked. */ - tablet_set_mode(0); + tablet_set_mode(0, TABLET_TRIGGER_LID); while (index < kAccelerometerVerticalHingeUnstableTestDataLength) { feed_accel_data(kAccelerometerVerticalHingeUnstableTestData, &index, filler); |