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>

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);