mkbp: EC buttons and switches via MKBP

Allowing EC buttons and switches to be signaled via MKBP protocol, using
CONFIG_MKBP_INPUT_DEVICES. Default behaviour is unchanged.

BUG=b:170966461
BRANCH=main,firmware-dedede-13606.B,firmware-volteer-13672.B-main
TEST=None

Signed-off-by: Boris Mittelberg <bmbm@google.com>
Cq-Depend: chromium:2824044
Change-Id: Ib96f98ecb3717a8ee8963be69fb7d7eb72e6d132
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2796382
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3700698
Commit-Queue: Shelley Chen <shchen@chromium.org>
Tested-by: Shelley Chen <shchen@chromium.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>

9 files changed, 245 insertions, 167 deletions

diff --git a/common/build.mk b/common/build.mk
index a38dbb24ac..4c506f1ca8 100644
--- a/common/build.mk
+++ b/common/build.mk
@@ -96,6 +96,8 @@ common-$(CONFIG_KEYBOARD_PROTOCOL_MKBP)+=keyboard_mkbp.o mkbp_fifo.o \
 	mkbp_info.o
 common-$(CONFIG_KEYBOARD_TEST)+=keyboard_test.o
 common-$(CONFIG_KEYBOARD_VIVALDI)+=keyboard_vivaldi.o
+common-$(CONFIG_MKBP_INPUT_DEVICES)+=mkbp_input_devices.o mkbp_fifo.o \
+	mkbp_info.o
 common-$(CONFIG_LED_COMMON)+=led_common.o
 common-$(CONFIG_LED_POLICY_STD)+=led_policy_std.o
 common-$(CONFIG_LED_PWM)+=led_pwm.o
diff --git a/common/button.c b/common/button.c
index 7eee2e41e1..c5dc412ce3 100644
--- a/common/button.c
+++ b/common/button.c
@@ -15,6 +15,7 @@
 #include "hooks.h"
 #include "keyboard_protocol.h"
 #include "led_common.h"
+#include "mkbp_input_devices.h"
 #include "power_button.h"
 #include "system.h"
 #include "timer.h"
@@ -251,10 +252,13 @@ static void button_change_deferred(void)
 				CPRINTS("Button '%s' was %s",
 					buttons[i].name, new_pressed ?
 					"pressed" : "released");
-#if defined(HAS_TASK_KEYPROTO) || defined(CONFIG_KEYBOARD_PROTOCOL_MKBP)
-				keyboard_update_button(buttons[i].type,
-					new_pressed);
-#endif
+				if (IS_ENABLED(CONFIG_MKBP_INPUT_DEVICES)) {
+					mkbp_button_update(buttons[i].type,
+								new_pressed);
+				} else if (IS_ENABLED(HAS_TASK_KEYPROTO)) {
+					keyboard_update_button(buttons[i].type,
+								new_pressed);
+				}
 			}
 
 			/* Clear the debounce time to stop checking it */
diff --git a/common/keyboard_8042.c b/common/keyboard_8042.c
index 512bde89dd..d845981993 100644
--- a/common/keyboard_8042.c
+++ b/common/keyboard_8042.c
@@ -1172,6 +1172,7 @@ static void keyboard_restore_state(void)
 }
 DECLARE_HOOK(HOOK_INIT, keyboard_restore_state, HOOK_PRIO_DEFAULT);
 
+#if defined(CONFIG_POWER_BUTTON) && !defined(CONFIG_MKBP_INPUT_DEVICES)
 /**
  * Handle power button changing state.
  */
@@ -1182,3 +1183,5 @@ static void keyboard_power_button(void)
 }
 DECLARE_HOOK(HOOK_POWER_BUTTON_CHANGE, keyboard_power_button,
 	     HOOK_PRIO_DEFAULT);
+
+#endif /* CONFIG_POWER_BUTTON && !CONFIG_MKBP_INPUT_DEVICES */
diff --git a/common/keyboard_mkbp.c b/common/keyboard_mkbp.c
index 159a0477a4..9baf70ee26 100644
--- a/common/keyboard_mkbp.c
+++ b/common/keyboard_mkbp.c
@@ -5,15 +5,8 @@
  * MKBP keyboard protocol
  */
 
-#include "atomic.h"
-#include "base_state.h"
-#include "button.h"
 #include "chipset.h"
 #include "common.h"
-#include "console.h"
-#include "ec_commands.h"
-#include "gpio.h"
-#include "hooks.h"
 #include "host_command.h"
 #include "keyboard_config.h"
 #include "keyboard_mkbp.h"
@@ -21,14 +14,9 @@
 #include "keyboard_raw.h"
 #include "keyboard_scan.h"
 #include "keyboard_test.h"
-#include "lid_switch.h"
 #include "mkbp_event.h"
 #include "mkbp_fifo.h"
-#include "power_button.h"
-#include "system.h"
-#include "tablet_mode.h"
 #include "task.h"
-#include "timer.h"
 #include "util.h"
 
 /* Console output macros */
@@ -42,9 +30,6 @@
 #define BATTERY_KEY_ROW 7
 #define BATTERY_KEY_ROW_MASK BIT(BATTERY_KEY_ROW)
 
-/* Button and switch state. */
-static uint32_t mkbp_button_state;
-static uint32_t mkbp_switch_state;
 #ifndef HAS_TASK_KEYSCAN
 /* Keys simulated-pressed */
 static uint8_t __bss_slow simulated_key[KEYBOARD_COLS_MAX];
@@ -71,17 +56,6 @@ static struct ec_mkbp_protocol_config config = {
 	.fifo_max_depth = FIFO_DEPTH,
 };
 
-uint32_t mkbp_get_switch_state(void)
-{
-	return mkbp_switch_state;
-};
-
-uint32_t mkbp_get_button_state(void)
-{
-	return mkbp_button_state;
-};
-
-
 /*****************************************************************************/
 /* Interface */
 
@@ -102,130 +76,12 @@ test_mockable int mkbp_keyboard_add(const uint8_t *buffp)
 	return mkbp_fifo_add((uint8_t)EC_MKBP_EVENT_KEY_MATRIX, buffp);
 }
 
-void mkbp_update_switches(uint32_t sw, int state)
-{
-
-	mkbp_switch_state &= ~BIT(sw);
-	mkbp_switch_state |= (!!state << sw);
-
-	mkbp_fifo_add(EC_MKBP_EVENT_SWITCH,
-		      (const uint8_t *)&mkbp_switch_state);
-}
-
-#ifdef CONFIG_LID_SWITCH
-/**
- * Handle lid changing state.
- */
-static void mkbp_lid_change(void)
-{
-	mkbp_update_switches(EC_MKBP_LID_OPEN, lid_is_open());
-}
-DECLARE_HOOK(HOOK_LID_CHANGE, mkbp_lid_change, HOOK_PRIO_LAST);
-DECLARE_HOOK(HOOK_INIT, mkbp_lid_change, HOOK_PRIO_INIT_LID+1);
-#endif
-
-#ifdef CONFIG_TABLET_MODE_SWITCH
-static void mkbp_tablet_mode_change(void)
-{
-	mkbp_update_switches(EC_MKBP_TABLET_MODE, tablet_get_mode());
-}
-DECLARE_HOOK(HOOK_TABLET_MODE_CHANGE, mkbp_tablet_mode_change, HOOK_PRIO_LAST);
-DECLARE_HOOK(HOOK_INIT, mkbp_tablet_mode_change, HOOK_PRIO_INIT_LID+1);
-#endif
-
-#ifdef CONFIG_BASE_ATTACHED_SWITCH
-static void mkbp_base_attached_change(void)
-{
-	mkbp_update_switches(EC_MKBP_BASE_ATTACHED, base_get_state());
-}
-DECLARE_HOOK(HOOK_BASE_ATTACHED_CHANGE, mkbp_base_attached_change,
-	     HOOK_PRIO_LAST);
-DECLARE_HOOK(HOOK_INIT, mkbp_base_attached_change, HOOK_PRIO_INIT_LID+1);
-#endif
-
-void keyboard_update_button(enum keyboard_button_type button, int is_pressed)
-{
-	switch (button) {
-	case KEYBOARD_BUTTON_POWER:
-		mkbp_button_state &= ~BIT(EC_MKBP_POWER_BUTTON);
-		mkbp_button_state |= (is_pressed << EC_MKBP_POWER_BUTTON);
-		break;
-
-	case KEYBOARD_BUTTON_VOLUME_UP:
-		mkbp_button_state &= ~BIT(EC_MKBP_VOL_UP);
-		mkbp_button_state |= (is_pressed << EC_MKBP_VOL_UP);
-		break;
-
-	case KEYBOARD_BUTTON_VOLUME_DOWN:
-		mkbp_button_state &= ~BIT(EC_MKBP_VOL_DOWN);
-		mkbp_button_state |= (is_pressed << EC_MKBP_VOL_DOWN);
-		break;
-
-	case KEYBOARD_BUTTON_RECOVERY:
-		mkbp_button_state &= ~BIT(EC_MKBP_RECOVERY);
-		mkbp_button_state |= (is_pressed << EC_MKBP_RECOVERY);
-		break;
-
-	default:
-		/* ignored. */
-		return;
-	}
-
-	CPRINTS("buttons: %x", mkbp_button_state);
-
-	/* Add the new state to the FIFO. */
-	mkbp_fifo_add(EC_MKBP_EVENT_BUTTON,
-		      (const uint8_t *)&mkbp_button_state);
-}
-
-#ifdef CONFIG_EMULATED_SYSRQ
-void host_send_sysrq(uint8_t key)
-{
-	uint32_t value = key;
-
-	mkbp_fifo_add(EC_MKBP_EVENT_SYSRQ, (const uint8_t *)&value);
-}
-#endif
-
-#ifdef CONFIG_POWER_BUTTON
-/**
- * Handle power button changing state.
- */
-static void keyboard_power_button(void)
-{
-	keyboard_update_button(KEYBOARD_BUTTON_POWER,
-			       power_button_is_pressed());
-}
-DECLARE_HOOK(HOOK_POWER_BUTTON_CHANGE, keyboard_power_button,
-	     HOOK_PRIO_DEFAULT);
-#endif /* defined(CONFIG_POWER_BUTTON) */
-
 static int keyboard_get_next_event(uint8_t *out)
 {
 	return mkbp_fifo_get_next_event(out, EC_MKBP_EVENT_KEY_MATRIX);
 }
 DECLARE_EVENT_SOURCE(EC_MKBP_EVENT_KEY_MATRIX, keyboard_get_next_event);
 
-static int button_get_next_event(uint8_t *out)
-{
-	return mkbp_fifo_get_next_event(out, EC_MKBP_EVENT_BUTTON);
-}
-DECLARE_EVENT_SOURCE(EC_MKBP_EVENT_BUTTON, button_get_next_event);
-
-static int switch_get_next_event(uint8_t *out)
-{
-	return mkbp_fifo_get_next_event(out, EC_MKBP_EVENT_SWITCH);
-}
-DECLARE_EVENT_SOURCE(EC_MKBP_EVENT_SWITCH, switch_get_next_event);
-
-#ifdef CONFIG_EMULATED_SYSRQ
-static int sysrq_get_next_event(uint8_t *out)
-{
-	return mkbp_fifo_get_next_event(out, EC_MKBP_EVENT_SYSRQ);
-}
-DECLARE_EVENT_SOURCE(EC_MKBP_EVENT_SYSRQ, sysrq_get_next_event);
-#endif
-
 void keyboard_send_battery_key(void)
 {
 	uint8_t state[KEYBOARD_COLS_MAX];
diff --git a/common/mkbp_info.c b/common/mkbp_info.c
index f31852abca..b3835367cf 100644
--- a/common/mkbp_info.c
+++ b/common/mkbp_info.c
@@ -12,6 +12,7 @@
 #include "keyboard_config.h"
 #include "keyboard_mkbp.h"
 #include "keyboard_scan.h"
+#include "mkbp_input_devices.h"
 #include "util.h"
 
 static uint32_t get_supported_buttons(void)
@@ -116,6 +117,7 @@ static enum ec_status mkbp_get_info(struct host_cmd_handler_args *args)
 				break;
 #endif
 
+#ifdef CONFIG_MKBP_INPUT_DEVICES
 			case EC_MKBP_EVENT_BUTTON:
 				r->buttons = mkbp_get_button_state();
 				args->response_size = sizeof(r->buttons);
@@ -125,6 +127,7 @@ static enum ec_status mkbp_get_info(struct host_cmd_handler_args *args)
 				r->switches = mkbp_get_switch_state();
 				args->response_size = sizeof(r->switches);
 				break;
+#endif /* CONFIG_MKBP_INPUT_DEVICES */
 
 			default:
 				/* Doesn't make sense for other event types. */
diff --git a/common/mkbp_input_devices.c b/common/mkbp_input_devices.c
new file mode 100644
index 0000000000..56d2234775
--- /dev/null
+++ b/common/mkbp_input_devices.c
@@ -0,0 +1,178 @@
+/* Copyright 2021 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+/* Input devices using Matrix Keyboard Protocol [MKBP] events for Chrome EC */
+
+#include "base_state.h"
+#include "button.h"
+#include "console.h"
+#include "hooks.h"
+#include "host_command.h"
+#include "keyboard_mkbp.h"
+#include "lid_switch.h"
+#include "mkbp_event.h"
+#include "mkbp_fifo.h"
+#include "mkbp_input_devices.h"
+#include "power_button.h"
+#include "tablet_mode.h"
+#include "util.h"
+
+#define CPRINTS(format, args...) cprints(CC_KEYBOARD, format, ## args)
+
+/* Buttons and switch state. */
+static uint32_t mkbp_button_state;
+static uint32_t mkbp_switch_state;
+
+static bool mkbp_init_done;
+
+uint32_t mkbp_get_switch_state(void)
+{
+	return mkbp_switch_state;
+};
+
+uint32_t mkbp_get_button_state(void)
+{
+	return mkbp_button_state;
+};
+
+void mkbp_button_update(enum keyboard_button_type button, int is_pressed)
+{
+	switch (button) {
+	case KEYBOARD_BUTTON_POWER:
+		mkbp_button_state &= ~BIT(EC_MKBP_POWER_BUTTON);
+		mkbp_button_state |= (is_pressed << EC_MKBP_POWER_BUTTON);
+		break;
+
+	case KEYBOARD_BUTTON_VOLUME_UP:
+		mkbp_button_state &= ~BIT(EC_MKBP_VOL_UP);
+		mkbp_button_state |= (is_pressed << EC_MKBP_VOL_UP);
+		break;
+
+	case KEYBOARD_BUTTON_VOLUME_DOWN:
+		mkbp_button_state &= ~BIT(EC_MKBP_VOL_DOWN);
+		mkbp_button_state |= (is_pressed << EC_MKBP_VOL_DOWN);
+		break;
+
+	case KEYBOARD_BUTTON_RECOVERY:
+		mkbp_button_state &= ~BIT(EC_MKBP_RECOVERY);
+		mkbp_button_state |= (is_pressed << EC_MKBP_RECOVERY);
+		break;
+
+	default:
+		/* ignored. */
+		return;
+	}
+
+	CPRINTS("mkbp buttons: %x", mkbp_button_state);
+
+	mkbp_fifo_add(EC_MKBP_EVENT_BUTTON,
+			(const uint8_t *)&mkbp_button_state);
+};
+
+void mkbp_update_switches(uint32_t sw, int state)
+{
+	mkbp_switch_state &= ~BIT(sw);
+	mkbp_switch_state |= (!!state << sw);
+
+	CPRINTS("mkbp switches: %x", mkbp_switch_state);
+
+	/*
+	 * Only inform AP mkbp changes when all switches initialized, in case
+	 * of the middle states causing the weird behaviour in the AP side,
+	 * especially when sysjumped while AP up.
+	 */
+	if (mkbp_init_done)
+		mkbp_fifo_add(EC_MKBP_EVENT_SWITCH,
+				(const uint8_t *)&mkbp_switch_state);
+}
+
+
+/*****************************************************************************/
+/* Hooks */
+
+#ifdef CONFIG_POWER_BUTTON
+/**
+ * Handle power button changing state.
+ */
+static void keyboard_power_button(void)
+{
+	mkbp_button_update(KEYBOARD_BUTTON_POWER,
+			       power_button_is_pressed());
+}
+DECLARE_HOOK(HOOK_POWER_BUTTON_CHANGE, keyboard_power_button,
+	     HOOK_PRIO_DEFAULT);
+#endif /* defined(CONFIG_POWER_BUTTON) */
+
+#ifdef CONFIG_LID_SWITCH
+/**
+ * Handle lid changing state.
+ */
+static void mkbp_lid_change(void)
+{
+	mkbp_update_switches(EC_MKBP_LID_OPEN, lid_is_open());
+}
+DECLARE_HOOK(HOOK_LID_CHANGE, mkbp_lid_change, HOOK_PRIO_LAST);
+DECLARE_HOOK(HOOK_INIT, mkbp_lid_change, HOOK_PRIO_INIT_LID+1);
+#endif
+
+#ifdef CONFIG_TABLET_MODE_SWITCH
+static void mkbp_tablet_mode_change(void)
+{
+	mkbp_update_switches(EC_MKBP_TABLET_MODE, tablet_get_mode());
+}
+DECLARE_HOOK(HOOK_TABLET_MODE_CHANGE, mkbp_tablet_mode_change, HOOK_PRIO_LAST);
+DECLARE_HOOK(HOOK_INIT, mkbp_tablet_mode_change, HOOK_PRIO_INIT_LID+1);
+#endif
+
+#ifdef CONFIG_BASE_ATTACHED_SWITCH
+static void mkbp_base_attached_change(void)
+{
+	mkbp_update_switches(EC_MKBP_BASE_ATTACHED, base_get_state());
+}
+DECLARE_HOOK(HOOK_BASE_ATTACHED_CHANGE, mkbp_base_attached_change,
+	     HOOK_PRIO_LAST);
+DECLARE_HOOK(HOOK_INIT, mkbp_base_attached_change, HOOK_PRIO_INIT_LID+1);
+#endif
+
+static void mkbp_report_switch_on_init(void)
+{
+	/* All switches initialized, report switch state to AP */
+	mkbp_init_done = true;
+	mkbp_fifo_add(EC_MKBP_EVENT_SWITCH,
+			(const uint8_t *)&mkbp_switch_state);
+}
+DECLARE_HOOK(HOOK_INIT, mkbp_report_switch_on_init, HOOK_PRIO_LAST);
+
+#ifdef CONFIG_EMULATED_SYSRQ
+void host_send_sysrq(uint8_t key)
+{
+	uint32_t value = key;
+
+	mkbp_fifo_add(EC_MKBP_EVENT_SYSRQ, (const uint8_t *)&value);
+}
+#endif
+
+/*****************************************************************************/
+/* Events */
+
+static int mkbp_button_get_next_event(uint8_t *out)
+{
+	return mkbp_fifo_get_next_event(out, EC_MKBP_EVENT_BUTTON);
+}
+DECLARE_EVENT_SOURCE(EC_MKBP_EVENT_BUTTON, mkbp_button_get_next_event);
+
+static int switch_get_next_event(uint8_t *out)
+{
+	return mkbp_fifo_get_next_event(out, EC_MKBP_EVENT_SWITCH);
+}
+DECLARE_EVENT_SOURCE(EC_MKBP_EVENT_SWITCH, switch_get_next_event);
+
+#ifdef CONFIG_EMULATED_SYSRQ
+static int sysrq_get_next_event(uint8_t *out)
+{
+	return mkbp_fifo_get_next_event(out, EC_MKBP_EVENT_SYSRQ);
+}
+DECLARE_EVENT_SOURCE(EC_MKBP_EVENT_SYSRQ, sysrq_get_next_event);
+#endif
diff --git a/include/config.h b/include/config.h
index ce09a03c85..a7a69933b0 100644
--- a/include/config.h
+++ b/include/config.h
@@ -2834,6 +2834,11 @@
  */
 #undef CONFIG_MKBP_EVENT_WAKEUP_MASK
 
+/*
+ * Send button, switch and sysrq events via MKBP protocol to the host.
+ */
+#undef CONFIG_MKBP_INPUT_DEVICES
+
 /* Support memory protection unit (MPU) */
 #undef CONFIG_MPU
 
@@ -4628,8 +4633,12 @@
 
 
 /******************************************************************************/
-/* The Matrix Keyboard Protocol depends on MKBP events. */
+/* The Matrix Keyboard Protocol depends on MKBP input devices and events. */
 #ifdef CONFIG_KEYBOARD_PROTOCOL_MKBP
+#define CONFIG_MKBP_INPUT_DEVICES
+#endif
+
+#if defined(CONFIG_KEYBOARD_PROTOCOL_MKBP) || defined(CONFIG_MKBP_INPUT_DEVICES)
 #define CONFIG_MKBP_EVENT
 #endif
 
diff --git a/include/keyboard_mkbp.h b/include/keyboard_mkbp.h
index 41c7357259..3d153d63b5 100644
--- a/include/keyboard_mkbp.h
+++ b/include/keyboard_mkbp.h
@@ -27,22 +27,4 @@ void keyboard_send_battery_key(void);
 static inline void keyboard_send_battery_key(void) { }
 #endif
 
-/**
- * Update the state of the switches.
- *
- * @param sw		The switch that changed.
- * @param state		The state of the switch.
- */
-void mkbp_update_switches(uint32_t sw, int state);
-
-/**
- * Retrieve state of buttons [Power, Volume up/down, etc]
- */
-uint32_t mkbp_get_button_state(void);
-
-/**
- * Retrieve state of switches [Lid open/closed, tablet mode switch, etc]
- */
-uint32_t mkbp_get_switch_state(void);
-
 #endif  /* __CROS_EC_KEYBOARD_MKBP_H */
diff --git a/include/mkbp_input_devices.h b/include/mkbp_input_devices.h
new file mode 100644
index 0000000000..89d567fdfc
--- /dev/null
+++ b/include/mkbp_input_devices.h
@@ -0,0 +1,41 @@
+/* Copyright 2021 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+/* Input devices using Matrix Keyboard Protocol [MKBP] events for Chrome EC */
+
+#ifndef __CROS_EC_MKBP_INPUT_DEVICES_H
+#define __CROS_EC_MKBP_INPUT_DEVICES_H
+
+#include "common.h"
+#include "button.h"
+#include "ec_commands.h"
+
+/**
+ * Update the state of the switches.
+ *
+ * @param sw		The switch that changed.
+ * @param state		The state of the switch.
+ */
+void mkbp_update_switches(uint32_t sw, int state);
+
+/**
+ * Update the state of buttons
+ *
+ * @param button	The button that changed.
+ * @param is_pressed	Whether the button is now pressed.
+ */
+void mkbp_button_update(enum keyboard_button_type button, int is_pressed);
+
+/**
+ * Retrieve state of buttons [Power, Volume up/down, etc]
+ */
+uint32_t mkbp_get_button_state(void);
+
+/**
+ * Retrieve state of switches [Lid open/closed, tablet mode switch, etc]
+ */
+uint32_t mkbp_get_switch_state(void);
+
+#endif /* __CROS_EC_MKBP_INPUT_DEVICES_H */