[PATCH 1/7] Input: export input_default_setkeycode

[PATCH 1/7] Input: export input_default_setkeycode

[Dmitry Torokhov]

From: Fabio Baltieri <fabiobaltieri@chromium.org>

Export input_default_setkeycode so that a driver can set a custom
setkeycode handler to take some driver specific action but still call
the default handler at some point.

Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
Link: https://patch.msgid.link/20260211173421.1206478-2-fabiobaltieri@chromium.org
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/input.c | 23 ++++++++++++++++++++---
 include/linux/input.h |  4 ++++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index ceb134044175..9336980ee320 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -861,14 +861,30 @@ static int input_default_getkeycode(struct input_dev *dev,
 	return 0;
 }
 
-static int input_default_setkeycode(struct input_dev *dev,
-				    const struct input_keymap_entry *ke,
-				    unsigned int *old_keycode)
+/**
+ * input_default_setkeycode - default setkeycode method
+ * @dev: input device which keymap is being updated.
+ * @ke: new keymap entry.
+ * @old_keycode: pointer to the location where old keycode should be stored.
+ *
+ * This function is the default implementation of &input_dev.setkeycode()
+ * method. It is typically used when a driver does not provide its own
+ * implementation, but it is also exported so drivers can extend it.
+ *
+ * The function must be called with &input_dev.event_lock held.
+ *
+ * Return: 0 on success, or a negative error code on failure.
+ */
+int input_default_setkeycode(struct input_dev *dev,
+			     const struct input_keymap_entry *ke,
+			     unsigned int *old_keycode)
 {
 	unsigned int index;
 	int error;
 	int i;
 
+	lockdep_assert_held(&dev->event_lock);
+
 	if (!dev->keycodesize)
 		return -EINVAL;
 
@@ -922,6 +938,7 @@ static int input_default_setkeycode(struct input_dev *dev,
 	__set_bit(ke->keycode, dev->keybit);
 	return 0;
 }
+EXPORT_SYMBOL(input_default_setkeycode);
 
 /**
  * input_get_keycode - retrieve keycode currently mapped to a given scancode
diff --git a/include/linux/input.h b/include/linux/input.h
index 7d7cb0593a63..06ca62328db1 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -517,6 +517,10 @@ INPUT_GENERATE_ABS_ACCESSORS(res, resolution)
 int input_scancode_to_scalar(const struct input_keymap_entry *ke,
 			     unsigned int *scancode);
 
+int input_default_setkeycode(struct input_dev *dev,
+			     const struct input_keymap_entry *ke,
+			     unsigned int *old_keycode);
+
 int input_get_keycode(struct input_dev *dev, struct input_keymap_entry *ke);
 int input_set_keycode(struct input_dev *dev,
 		      const struct input_keymap_entry *ke);
-- 
2.53.0.345.g96ddfc5eaa-goog

[PATCH 2/7] Input: cros_ec_keyb - add function key support

[Dmitry Torokhov]

From: Fabio Baltieri <fabiobaltieri@chromium.org>

Add support for handling an Fn button and sending separate keycodes for
a subset of keys in the matrix defined in the upper half of the keymap.

Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Link: https://patch.msgid.link/20260211173421.1206478-3-fabiobaltieri@chromium.org
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/cros_ec_keyb.c | 182 +++++++++++++++++++++++---
 1 file changed, 166 insertions(+), 16 deletions(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 066aaa87d93f..bdbfd219f2e8 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -29,6 +29,12 @@
 
 #include <linux/unaligned.h>
 
+/*
+ * Maximum size of the normal key matrix, this is limited by the host command
+ * key_matrix field defined in ec_response_get_next_data_v3
+ */
+#define CROS_EC_KEYBOARD_COLS_MAX 18
+
 /**
  * struct cros_ec_keyb - Structure representing EC keyboard device
  *
@@ -44,6 +50,9 @@
  * @bs_idev: The input device for non-matrix buttons and switches (or NULL).
  * @notifier: interrupt event notifier for transport devices
  * @vdata: vivaldi function row data
+ * @has_fn_map: whether the driver uses an fn function-map layer
+ * @fn_active: tracks whether the function key is currently pressed
+ * @fn_combo_active: tracks whether another key was pressed while fn is active
  */
 struct cros_ec_keyb {
 	unsigned int rows;
@@ -61,6 +70,10 @@ struct cros_ec_keyb {
 	struct notifier_block notifier;
 
 	struct vivaldi_data vdata;
+
+	bool has_fn_map;
+	bool fn_active;
+	bool fn_combo_active;
 };
 
 /**
@@ -166,16 +179,89 @@ static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
 	return false;
 }
 
+static void cros_ec_emit_fn_key(struct input_dev *input, unsigned int pos)
+{
+	input_event(input, EV_MSC, MSC_SCAN, pos);
+	input_report_key(input, KEY_FN, true);
+	input_sync(input);
+
+	input_event(input, EV_MSC, MSC_SCAN, pos);
+	input_report_key(input, KEY_FN, false);
+}
+
+static void cros_ec_keyb_process_key_plain(struct cros_ec_keyb *ckdev,
+					   int row, int col, bool state)
+{
+	struct input_dev *idev = ckdev->idev;
+	const unsigned short *keycodes = idev->keycode;
+	int pos = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
+
+	input_event(idev, EV_MSC, MSC_SCAN, pos);
+	input_report_key(idev, keycodes[pos], state);
+}
+
+static void cros_ec_keyb_process_key_fn_map(struct cros_ec_keyb *ckdev,
+					    int row, int col, bool state)
+{
+	struct input_dev *idev = ckdev->idev;
+	const unsigned short *keycodes = idev->keycode;
+	unsigned int pos, fn_pos;
+	unsigned int code, fn_code;
+
+	pos = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
+	code = keycodes[pos];
+
+	if (code == KEY_FN) {
+		ckdev->fn_active = state;
+		if (state) {
+			ckdev->fn_combo_active = false;
+		} else if (!ckdev->fn_combo_active) {
+			/*
+			 * Send both Fn press and release events if nothing
+			 * else has been pressed together with Fn.
+			 */
+			cros_ec_emit_fn_key(idev, pos);
+		}
+		return;
+	}
+
+	fn_pos = MATRIX_SCAN_CODE(row + ckdev->rows, col, ckdev->row_shift);
+	fn_code = keycodes[fn_pos];
+
+	if (state) {
+		if (ckdev->fn_active) {
+			ckdev->fn_combo_active = true;
+			if (!fn_code)
+				return; /* Discard if no Fn mapping exists */
+
+			code = fn_code;
+			pos = fn_pos;
+		}
+	} else {
+		/*
+		 * If the Fn-remapped code is currently pressed, release it.
+		 * Otherwise, release the standard code (if it was pressed).
+		 */
+		if (fn_code && test_bit(fn_code, idev->key)) {
+			code = fn_code;
+			pos = fn_pos;
+		} else if (!test_bit(code, idev->key)) {
+			return; /* Discard, key press code was not sent */
+		}
+	}
+
+	input_event(idev, EV_MSC, MSC_SCAN, pos);
+	input_report_key(idev, code, state);
+}
 
 /*
  * Compares the new keyboard state to the old one and produces key
- * press/release events accordingly.  The keyboard state is 13 bytes (one byte
- * per column)
+ * press/release events accordingly.  The keyboard state is one byte
+ * per column.
  */
 static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
 			 uint8_t *kb_state, int len)
 {
-	struct input_dev *idev = ckdev->idev;
 	int col, row;
 	int new_state;
 	int old_state;
@@ -192,20 +278,19 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
 
 	for (col = 0; col < ckdev->cols; col++) {
 		for (row = 0; row < ckdev->rows; row++) {
-			int pos = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
-			const unsigned short *keycodes = idev->keycode;
-
 			new_state = kb_state[col] & (1 << row);
 			old_state = ckdev->old_kb_state[col] & (1 << row);
-			if (new_state != old_state) {
-				dev_dbg(ckdev->dev,
-					"changed: [r%d c%d]: byte %02x\n",
-					row, col, new_state);
-
-				input_event(idev, EV_MSC, MSC_SCAN, pos);
-				input_report_key(idev, keycodes[pos],
-						 new_state);
-			}
+
+			if (new_state == old_state)
+				continue;
+
+			dev_dbg(ckdev->dev, "changed: [r%d c%d]: byte %02x\n",
+				row, col, new_state);
+
+			if (ckdev->has_fn_map)
+				cros_ec_keyb_process_key_fn_map(ckdev, row, col, new_state);
+			else
+				cros_ec_keyb_process_key_plain(ckdev, row, col, new_state);
 		}
 		ckdev->old_kb_state[col] = kb_state[col];
 	}
@@ -583,6 +668,62 @@ static void cros_ec_keyb_parse_vivaldi_physmap(struct cros_ec_keyb *ckdev)
 	ckdev->vdata.num_function_row_keys = n_physmap;
 }
 
+/* Returns true if there is a KEY_FN code defined in the normal keymap */
+static bool cros_ec_keyb_has_fn_key(struct cros_ec_keyb *ckdev)
+{
+	const unsigned short *keycodes = ckdev->idev->keycode;
+	int i;
+
+	for (i = 0; i < MATRIX_SCAN_CODE(ckdev->rows, 0, ckdev->row_shift); i++) {
+		if (keycodes[i] == KEY_FN)
+			return true;
+	}
+
+	return false;
+}
+
+/*
+ * Returns true if there is a KEY_FN defined and at least one key in the fn
+ * layer keymap
+ */
+static bool cros_ec_keyb_has_fn_map(struct cros_ec_keyb *ckdev)
+{
+	struct input_dev *idev = ckdev->idev;
+	const unsigned short *keycodes = ckdev->idev->keycode;
+	int i;
+
+	if (!cros_ec_keyb_has_fn_key(ckdev))
+		return false;
+
+	for (i = MATRIX_SCAN_CODE(ckdev->rows, 0, ckdev->row_shift);
+	     i < idev->keycodemax; i++) {
+		if (keycodes[i] != KEY_RESERVED)
+			return true;
+	}
+
+	return false;
+}
+
+/*
+ * Custom handler for the set keycode ioctl, calls the default handler and
+ * recomputes has_fn_map.
+ */
+static int cros_ec_keyb_setkeycode(struct input_dev *idev,
+				   const struct input_keymap_entry *ke,
+				   unsigned int *old_keycode)
+{
+	struct cros_ec_keyb *ckdev = input_get_drvdata(idev);
+	int ret;
+
+	ret = input_default_setkeycode(idev, ke, old_keycode);
+	if (ret)
+		return ret;
+
+	ckdev->has_fn_map = cros_ec_keyb_has_fn_map(ckdev);
+
+	return 0;
+}
+
 /**
  * cros_ec_keyb_register_matrix - Register matrix keys
  *
@@ -604,6 +745,12 @@ static int cros_ec_keyb_register_matrix(struct cros_ec_keyb *ckdev)
 	if (err)
 		return err;
 
+	if (ckdev->cols > CROS_EC_KEYBOARD_COLS_MAX) {
+		dev_err(dev, "keypad,num-columns too large: %d (max: %d)\n",
+			ckdev->cols, CROS_EC_KEYBOARD_COLS_MAX);
+		return -EINVAL;
+	}
+
 	ckdev->valid_keys = devm_kzalloc(dev, ckdev->cols, GFP_KERNEL);
 	if (!ckdev->valid_keys)
 		return -ENOMEM;
@@ -632,11 +779,12 @@ static int cros_ec_keyb_register_matrix(struct cros_ec_keyb *ckdev)
 	idev->id.version = 1;
 	idev->id.product = 0;
 	idev->dev.parent = dev;
+	idev->setkeycode = cros_ec_keyb_setkeycode;
 
 	ckdev->ghost_filter = device_property_read_bool(dev,
 					"google,needs-ghost-filter");
 
-	err = matrix_keypad_build_keymap(NULL, ckdev->rows, ckdev->cols,
+	err = matrix_keypad_build_keymap(NULL, ckdev->rows * 2, ckdev->cols,
 					 NULL, idev);
 	if (err) {
 		dev_err(dev, "cannot build key matrix\n");
@@ -651,6 +799,8 @@ static int cros_ec_keyb_register_matrix(struct cros_ec_keyb *ckdev)
 	cros_ec_keyb_compute_valid_keys(ckdev);
 	cros_ec_keyb_parse_vivaldi_physmap(ckdev);
 
+	ckdev->has_fn_map = cros_ec_keyb_has_fn_map(ckdev);
+
 	err = input_register_device(ckdev->idev);
 	if (err) {
 		dev_err(dev, "cannot register input device\n");
-- 
2.53.0.345.g96ddfc5eaa-goog

[PATCH 3/7] Input: cros_ec_keyb - use u8 instead of uint8_t

[Dmitry Torokhov]

In the kernel u8/u16/u32 are preferred to the uint*_t variants.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/cros_ec_keyb.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index bdbfd219f2e8..2f78a19521ab 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -59,8 +59,8 @@ struct cros_ec_keyb {
 	unsigned int cols;
 	int row_shift;
 	bool ghost_filter;
-	uint8_t *valid_keys;
-	uint8_t *old_kb_state;
+	u8 *valid_keys;
+	u8 *old_kb_state;
 
 	struct device *dev;
 	struct cros_ec_device *ec;
@@ -145,11 +145,11 @@ static const struct cros_ec_bs_map cros_ec_keyb_bs[] = {
  * Returns true when there is at least one combination of pressed keys that
  * results in ghosting.
  */
-static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
+static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, u8 *buf)
 {
 	int col1, col2, buf1, buf2;
 	struct device *dev = ckdev->dev;
-	uint8_t *valid_keys = ckdev->valid_keys;
+	u8 *valid_keys = ckdev->valid_keys;
 
 	/*
 	 * Ghosting happens if for any pressed key X there are other keys
@@ -259,8 +259,7 @@ static void cros_ec_keyb_process_key_fn_map(struct cros_ec_keyb *ckdev,
  * press/release events accordingly.  The keyboard state is one byte
  * per column.
  */
-static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
-			 uint8_t *kb_state, int len)
+static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev, u8 *kb_state, int len)
 {
 	int col, row;
 	int new_state;
-- 
2.53.0.345.g96ddfc5eaa-goog

[PATCH 4/7] Input: cros_ec_keyb - use BIT() macro instead of open-coding shifts

[Dmitry Torokhov]

Using the macro clarifies the code.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/cros_ec_keyb.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 2f78a19521ab..fabbdbc22785 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -277,8 +277,8 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev, u8 *kb_state, int l
 
 	for (col = 0; col < ckdev->cols; col++) {
 		for (row = 0; row < ckdev->rows; row++) {
-			new_state = kb_state[col] & (1 << row);
-			old_state = ckdev->old_kb_state[col] & (1 << row);
+			new_state = kb_state[col] & BIT(row);
+			old_state = ckdev->old_kb_state[col] & BIT(row);
 
 			if (new_state == old_state)
 				continue;
@@ -411,7 +411,7 @@ static void cros_ec_keyb_compute_valid_keys(struct cros_ec_keyb *ckdev)
 		for (row = 0; row < ckdev->rows; row++) {
 			code = keymap[MATRIX_SCAN_CODE(row, col, row_shift)];
 			if (code && (code != KEY_BATTERY))
-				ckdev->valid_keys[col] |= 1 << row;
+				ckdev->valid_keys[col] |= BIT(row);
 		}
 		dev_dbg(ckdev->dev, "valid_keys[%02d] = 0x%02x\n",
 			col, ckdev->valid_keys[col]);
@@ -780,8 +780,7 @@ static int cros_ec_keyb_register_matrix(struct cros_ec_keyb *ckdev)
 	idev->dev.parent = dev;
 	idev->setkeycode = cros_ec_keyb_setkeycode;
 
-	ckdev->ghost_filter = device_property_read_bool(dev,
-					"google,needs-ghost-filter");
+	ckdev->ghost_filter = device_property_read_bool(dev, "google,needs-ghost-filter");
 
 	err = matrix_keypad_build_keymap(NULL, ckdev->rows * 2, ckdev->cols,
 					 NULL, idev);
-- 
2.53.0.345.g96ddfc5eaa-goog

[PATCH 5/7] Input: cros_ec_keyb - simplify cros_ec_keyb_work()

[Dmitry Torokhov]

Introduce temporaries for event_data pointer and event_size to simplify
the code a bit.

In cros_ec_keyb_compute_valid_keys() explicitly compare with
KEY_RESERVED to make the intent of the comparison more clear.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/cros_ec_keyb.c | 28 +++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index fabbdbc22785..1b4ee30e1998 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -330,8 +330,10 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
 {
 	struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
 						  notifier);
-	u32 val;
+	struct ec_response_get_next_event_v3 *event_data;
+	unsigned int event_size;
 	unsigned int ev_type;
+	u32 val;
 
 	/*
 	 * If not wake enabled, discard key state changes during
@@ -341,32 +343,32 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
 	if (queued_during_suspend && !device_may_wakeup(ckdev->dev))
 		return NOTIFY_OK;
 
-	switch (ckdev->ec->event_data.event_type) {
+	event_data = &ckdev->ec->event_data;
+	event_size = ckdev->ec->event_size;
+
+	switch (event_data->event_type) {
 	case EC_MKBP_EVENT_KEY_MATRIX:
 		pm_wakeup_event(ckdev->dev, 0);
 
 		if (!ckdev->idev) {
-			dev_warn_once(ckdev->dev,
-				      "Unexpected key matrix event\n");
+			dev_warn_once(ckdev->dev, "Unexpected key matrix event\n");
 			return NOTIFY_OK;
 		}
 
-		if (ckdev->ec->event_size != ckdev->cols) {
+		if (event_size != ckdev->cols) {
 			dev_err(ckdev->dev,
 				"Discarded key matrix event, unexpected length: %d != %d\n",
 				ckdev->ec->event_size, ckdev->cols);
 			return NOTIFY_OK;
 		}
 
-		cros_ec_keyb_process(ckdev,
-				     ckdev->ec->event_data.data.key_matrix,
-				     ckdev->ec->event_size);
+		cros_ec_keyb_process(ckdev, event_data->data.key_matrix, event_size);
 		break;
 
 	case EC_MKBP_EVENT_SYSRQ:
 		pm_wakeup_event(ckdev->dev, 0);
 
-		val = get_unaligned_le32(&ckdev->ec->event_data.data.sysrq);
+		val = get_unaligned_le32(&event_data->data.sysrq);
 		dev_dbg(ckdev->dev, "sysrq code from EC: %#x\n", val);
 		handle_sysrq(val);
 		break;
@@ -376,12 +378,10 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
 		pm_wakeup_event(ckdev->dev, 0);
 
 		if (ckdev->ec->event_data.event_type == EC_MKBP_EVENT_BUTTON) {
-			val = get_unaligned_le32(
-					&ckdev->ec->event_data.data.buttons);
+			val = get_unaligned_le32(&event_data->data.buttons);
 			ev_type = EV_KEY;
 		} else {
-			val = get_unaligned_le32(
-					&ckdev->ec->event_data.data.switches);
+			val = get_unaligned_le32(&event_data->data.switches);
 			ev_type = EV_SW;
 		}
 		cros_ec_keyb_report_bs(ckdev, ev_type, val);
@@ -410,7 +410,7 @@ static void cros_ec_keyb_compute_valid_keys(struct cros_ec_keyb *ckdev)
 	for (col = 0; col < ckdev->cols; col++) {
 		for (row = 0; row < ckdev->rows; row++) {
 			code = keymap[MATRIX_SCAN_CODE(row, col, row_shift)];
-			if (code && (code != KEY_BATTERY))
+			if (code != KEY_RESERVED && code != KEY_BATTERY)
 				ckdev->valid_keys[col] |= BIT(row);
 		}
 		dev_dbg(ckdev->dev, "valid_keys[%02d] = 0x%02x\n",
-- 
2.53.0.345.g96ddfc5eaa-goog

[PATCH 6/7] Input: cros_ec_keyb - do not allocate keyboard state separately

[Dmitry Torokhov]

Now that we know the upper bound for the number of columnts, and know
that it is pretty small, there is no point in allocating it separately.
We are wasting more memory tracking the allocations.

Embed valid_keys and old_kb_state directly into cros_ec_keyb structure.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/cros_ec_keyb.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 1b4ee30e1998..02176aee0530 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -59,8 +59,8 @@ struct cros_ec_keyb {
 	unsigned int cols;
 	int row_shift;
 	bool ghost_filter;
-	u8 *valid_keys;
-	u8 *old_kb_state;
+	u8 valid_keys[CROS_EC_KEYBOARD_COLS_MAX];
+	u8 old_kb_state[CROS_EC_KEYBOARD_COLS_MAX];
 
 	struct device *dev;
 	struct cros_ec_device *ec;
@@ -750,14 +750,6 @@ static int cros_ec_keyb_register_matrix(struct cros_ec_keyb *ckdev)
 		return -EINVAL;
 	}
 
-	ckdev->valid_keys = devm_kzalloc(dev, ckdev->cols, GFP_KERNEL);
-	if (!ckdev->valid_keys)
-		return -ENOMEM;
-
-	ckdev->old_kb_state = devm_kzalloc(dev, ckdev->cols, GFP_KERNEL);
-	if (!ckdev->old_kb_state)
-		return -ENOMEM;
-
 	/*
 	 * We call the keyboard matrix 'input0'. Allocate phys before input
 	 * dev, to ensure correct tear-down ordering.
-- 
2.53.0.345.g96ddfc5eaa-goog

[PATCH 7/7] Input: cros_ec_keyb - factor out column processing

[Dmitry Torokhov]

Factor out column processing and eagerly skip processing columns that do
not have any changes in them.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/cros_ec_keyb.c | 47 +++++++++++++++------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 02176aee0530..8cf9129f6198 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -254,6 +254,26 @@ static void cros_ec_keyb_process_key_fn_map(struct cros_ec_keyb *ckdev,
 	input_report_key(idev, code, state);
 }
 
+static void cros_ec_keyy_process_col(struct cros_ec_keyb *ckdev, int col,
+				     u8 col_state, u8 changed)
+{
+	for (int row = 0; row < ckdev->rows; row++) {
+		if (changed & BIT(row)) {
+			u8 key_state = col_state & BIT(row);
+
+			dev_dbg(ckdev->dev, "changed: [r%d c%d]: byte %02x\n",
+				row, col, key_state);
+
+			if (ckdev->has_fn_map)
+				cros_ec_keyb_process_key_fn_map(ckdev, row, col,
+								key_state);
+			else
+				cros_ec_keyb_process_key_plain(ckdev, row, col,
+							       key_state);
+		}
+	}
+}
+
 /*
  * Compares the new keyboard state to the old one and produces key
  * press/release events accordingly.  The keyboard state is one byte
@@ -261,10 +281,6 @@ static void cros_ec_keyb_process_key_fn_map(struct cros_ec_keyb *ckdev,
  */
 static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev, u8 *kb_state, int len)
 {
-	int col, row;
-	int new_state;
-	int old_state;
-
 	if (ckdev->ghost_filter && cros_ec_keyb_has_ghosting(ckdev, kb_state)) {
 		/*
 		 * Simple-minded solution: ignore this state. The obvious
@@ -275,24 +291,15 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev, u8 *kb_state, int l
 		return;
 	}
 
-	for (col = 0; col < ckdev->cols; col++) {
-		for (row = 0; row < ckdev->rows; row++) {
-			new_state = kb_state[col] & BIT(row);
-			old_state = ckdev->old_kb_state[col] & BIT(row);
-
-			if (new_state == old_state)
-				continue;
-
-			dev_dbg(ckdev->dev, "changed: [r%d c%d]: byte %02x\n",
-				row, col, new_state);
+	for (int col = 0; col < ckdev->cols; col++) {
+		u8 changed = kb_state[col] ^ ckdev->old_kb_state[col];
 
-			if (ckdev->has_fn_map)
-				cros_ec_keyb_process_key_fn_map(ckdev, row, col, new_state);
-			else
-				cros_ec_keyb_process_key_plain(ckdev, row, col, new_state);
-		}
-		ckdev->old_kb_state[col] = kb_state[col];
+		if (changed)
+			cros_ec_keyy_process_col(ckdev, col, kb_state[col],
+						 changed);
 	}
+
+	memcpy(ckdev->old_kb_state, kb_state, sizeof(ckdev->old_kb_state));
 	input_sync(ckdev->idev);
 }
 
-- 
2.53.0.345.g96ddfc5eaa-goog

Re: [PATCH 1/7] Input: export input_default_setkeycode

[Tzung-Bi Shih]

On Sat, Feb 21, 2026 at 04:37:09PM -0800, Dmitry Torokhov wrote:
> From: Fabio Baltieri <fabiobaltieri@chromium.org>
> 
> Export input_default_setkeycode so that a driver can set a custom
> setkeycode handler to take some driver specific action but still call
> the default handler at some point.
> 
> Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> Link: https://patch.msgid.link/20260211173421.1206478-2-fabiobaltieri@chromium.org
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Re: [PATCH 2/7] Input: cros_ec_keyb - add function key support

[Tzung-Bi Shih]

On Sat, Feb 21, 2026 at 04:37:10PM -0800, Dmitry Torokhov wrote:
> From: Fabio Baltieri <fabiobaltieri@chromium.org>
> 
> Add support for handling an Fn button and sending separate keycodes for
> a subset of keys in the matrix defined in the upper half of the keymap.
> 
> Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Link: https://patch.msgid.link/20260211173421.1206478-3-fabiobaltieri@chromium.org
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

The patch looks good to me, just a few minor nits below.  I don't insist on
these being fixed for this patch to be merged,

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

> +static void cros_ec_keyb_process_key_fn_map(struct cros_ec_keyb *ckdev,
> +					    int row, int col, bool state)
> +{
> +	struct input_dev *idev = ckdev->idev;
> +	const unsigned short *keycodes = idev->keycode;
> +	unsigned int pos, fn_pos;
> +	unsigned int code, fn_code;

Nit: does declaring `code` and `fn_code` as unsigned short make more sense?
Or they can be declared in the same line.

> +
> +	pos = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
> +	code = keycodes[pos];
> +
> +	if (code == KEY_FN) {
> +		ckdev->fn_active = state;
> +		if (state) {
> +			ckdev->fn_combo_active = false;
> +		} else if (!ckdev->fn_combo_active) {
> +			/*
> +			 * Send both Fn press and release events if nothing
> +			 * else has been pressed together with Fn.
> +			 */
> +			cros_ec_emit_fn_key(idev, pos);
> +		}
> +		return;
> +	}
> +
> +	fn_pos = MATRIX_SCAN_CODE(row + ckdev->rows, col, ckdev->row_shift);
> +	fn_code = keycodes[fn_pos];
> +
> +	if (state) {
> +		if (ckdev->fn_active) {
> +			ckdev->fn_combo_active = true;
> +			if (!fn_code)
> +				return; /* Discard if no Fn mapping exists */
> +
> +			code = fn_code;
> +			pos = fn_pos;

Nit: assigning `pos` before `code` makes it neater.

> +		}
> +	} else {
> +		/*
> +		 * If the Fn-remapped code is currently pressed, release it.
> +		 * Otherwise, release the standard code (if it was pressed).
> +		 */
> +		if (fn_code && test_bit(fn_code, idev->key)) {
> +			code = fn_code;
> +			pos = fn_pos;

Nit: same here.

Re: [PATCH 3/7] Input: cros_ec_keyb - use u8 instead of uint8_t

[Tzung-Bi Shih]

On Sat, Feb 21, 2026 at 04:37:11PM -0800, Dmitry Torokhov wrote:
> In the kernel u8/u16/u32 are preferred to the uint*_t variants.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Re: [PATCH 4/7] Input: cros_ec_keyb - use BIT() macro instead of open-coding shifts

[Tzung-Bi Shih]

On Sat, Feb 21, 2026 at 04:37:12PM -0800, Dmitry Torokhov wrote:
> Using the macro clarifies the code.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

> @@ -780,8 +780,7 @@ static int cros_ec_keyb_register_matrix(struct cros_ec_keyb *ckdev)
>  	idev->dev.parent = dev;
>  	idev->setkeycode = cros_ec_keyb_setkeycode;
>  
> -	ckdev->ghost_filter = device_property_read_bool(dev,
> -					"google,needs-ghost-filter");
> +	ckdev->ghost_filter = device_property_read_bool(dev, "google,needs-ghost-filter");

This change seems unrelated to the patch, but it's fine as-is.  I don't think
it requires a separate commit.

Re: [PATCH 5/7] Input: cros_ec_keyb - simplify cros_ec_keyb_work()

[Tzung-Bi Shih]

On Sat, Feb 21, 2026 at 04:37:13PM -0800, Dmitry Torokhov wrote:
> @@ -376,12 +378,10 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
>  		pm_wakeup_event(ckdev->dev, 0);
>  
>  		if (ckdev->ec->event_data.event_type == EC_MKBP_EVENT_BUTTON) {

This can also be simplified: `event_data->event_type`.

Re: [PATCH 6/7] Input: cros_ec_keyb - do not allocate keyboard state separately

[Tzung-Bi Shih]

On Sat, Feb 21, 2026 at 04:37:14PM -0800, Dmitry Torokhov wrote:
> Now that we know the upper bound for the number of columnts, and know
> that it is pretty small, there is no point in allocating it separately.
> We are wasting more memory tracking the allocations.
> 
> Embed valid_keys and old_kb_state directly into cros_ec_keyb structure.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Re: [PATCH 7/7] Input: cros_ec_keyb - factor out column processing

[Tzung-Bi Shih]

On Sat, Feb 21, 2026 at 04:37:15PM -0800, Dmitry Torokhov wrote:
> +static void cros_ec_keyy_process_col(struct cros_ec_keyb *ckdev, int col,
> +				     u8 col_state, u8 changed)

What does the second 'y' of keyy stand for?

> +{
> +	for (int row = 0; row < ckdev->rows; row++) {
> +		if (changed & BIT(row)) {

Maybe

    if ((changed & BIT(row)) == 0)
        continue;
or
    if (~changed & BIT(row))
        continue;
or
    if (!test_bit(row, &changed))
        continue;

to save an indent level.

Re: [PATCH 2/7] Input: cros_ec_keyb - add function key support

[Dmitry Torokhov]

On Sun, Feb 22, 2026 at 06:46:53PM +0800, Tzung-Bi Shih wrote:
> On Sat, Feb 21, 2026 at 04:37:10PM -0800, Dmitry Torokhov wrote:
> > From: Fabio Baltieri <fabiobaltieri@chromium.org>
> > 
> > Add support for handling an Fn button and sending separate keycodes for
> > a subset of keys in the matrix defined in the upper half of the keymap.
> > 
> > Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > Link: https://patch.msgid.link/20260211173421.1206478-3-fabiobaltieri@chromium.org
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> The patch looks good to me, just a few minor nits below.  I don't insist on
> these being fixed for this patch to be merged,
> 
> Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Thanks Tzung-Bi.

> 
> > +static void cros_ec_keyb_process_key_fn_map(struct cros_ec_keyb *ckdev,
> > +					    int row, int col, bool state)
> > +{
> > +	struct input_dev *idev = ckdev->idev;
> > +	const unsigned short *keycodes = idev->keycode;
> > +	unsigned int pos, fn_pos;
> > +	unsigned int code, fn_code;
> 
> Nit: does declaring `code` and `fn_code` as unsigned short make more sense?
> Or they can be declared in the same line.

I do not think making temporary variables unsigned short changes
anything, so I will leave it as is.

> 
> > +
> > +	pos = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
> > +	code = keycodes[pos];
> > +
> > +	if (code == KEY_FN) {
> > +		ckdev->fn_active = state;
> > +		if (state) {
> > +			ckdev->fn_combo_active = false;
> > +		} else if (!ckdev->fn_combo_active) {
> > +			/*
> > +			 * Send both Fn press and release events if nothing
> > +			 * else has been pressed together with Fn.
> > +			 */
> > +			cros_ec_emit_fn_key(idev, pos);
> > +		}
> > +		return;
> > +	}
> > +
> > +	fn_pos = MATRIX_SCAN_CODE(row + ckdev->rows, col, ckdev->row_shift);
> > +	fn_code = keycodes[fn_pos];
> > +
> > +	if (state) {
> > +		if (ckdev->fn_active) {
> > +			ckdev->fn_combo_active = true;
> > +			if (!fn_code)
> > +				return; /* Discard if no Fn mapping exists */
> > +
> > +			code = fn_code;
> > +			pos = fn_pos;
> 
> Nit: assigning `pos` before `code` makes it neater.
> 
> > +		}
> > +	} else {
> > +		/*
> > +		 * If the Fn-remapped code is currently pressed, release it.
> > +		 * Otherwise, release the standard code (if it was pressed).
> > +		 */
> > +		if (fn_code && test_bit(fn_code, idev->key)) {
> > +			code = fn_code;
> > +			pos = fn_pos;
> 
> Nit: same here.

Yep, I'll update before applying.

Thanks.

-- 
Dmitry

Re: [PATCH 5/7] Input: cros_ec_keyb - simplify cros_ec_keyb_work()

[Dmitry Torokhov]

On Sun, Feb 22, 2026 at 06:47:46PM +0800, Tzung-Bi Shih wrote:
> On Sat, Feb 21, 2026 at 04:37:13PM -0800, Dmitry Torokhov wrote:
> > @@ -376,12 +378,10 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
> >  		pm_wakeup_event(ckdev->dev, 0);
> >  
> >  		if (ckdev->ec->event_data.event_type == EC_MKBP_EVENT_BUTTON) {
> 
> This can also be simplified: `event_data->event_type`.

Indeed, will update.

Thanks.

-- 
Dmitry

Re: [PATCH 7/7] Input: cros_ec_keyb - factor out column processing

[Dmitry Torokhov]

On Sun, Feb 22, 2026 at 06:48:18PM +0800, Tzung-Bi Shih wrote:
> On Sat, Feb 21, 2026 at 04:37:15PM -0800, Dmitry Torokhov wrote:
> > +static void cros_ec_keyy_process_col(struct cros_ec_keyb *ckdev, int col,
> > +				     u8 col_state, u8 changed)
> 
> What does the second 'y' of keyy stand for?

It was supposed to be "keyb", I'll fix it before applying.

> 
> > +{
> > +	for (int row = 0; row < ckdev->rows; row++) {
> > +		if (changed & BIT(row)) {
> 
> Maybe
> 
>     if ((changed & BIT(row)) == 0)
>         continue;
> or
>     if (~changed & BIT(row))
>         continue;
> or
>     if (!test_bit(row, &changed))
>         continue;
> 
> to save an indent level.

I do not think indent is too deep here so we can't spare one level...

Thanks.

-- 
Dmitry

Re: [PATCH 7/7] Input: cros_ec_keyb - factor out column processing

[Tzung-Bi Shih]

On Sun, Feb 22, 2026 at 07:51:10PM -0800, Dmitry Torokhov wrote:
> On Sun, Feb 22, 2026 at 06:48:18PM +0800, Tzung-Bi Shih wrote:
> > On Sat, Feb 21, 2026 at 04:37:15PM -0800, Dmitry Torokhov wrote:
> > > +static void cros_ec_keyy_process_col(struct cros_ec_keyb *ckdev, int col,
> > > +				     u8 col_state, u8 changed)
> > 
> > What does the second 'y' of keyy stand for?
> 
> It was supposed to be "keyb", I'll fix it before applying.

Feel free to use my R-b tag after applying the change:
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Re: [PATCH 5/7] Input: cros_ec_keyb - simplify cros_ec_keyb_work()

[Tzung-Bi Shih]

On Sun, Feb 22, 2026 at 07:49:54PM -0800, Dmitry Torokhov wrote:
> On Sun, Feb 22, 2026 at 06:47:46PM +0800, Tzung-Bi Shih wrote:
> > On Sat, Feb 21, 2026 at 04:37:13PM -0800, Dmitry Torokhov wrote:
> > > @@ -376,12 +378,10 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
> > >  		pm_wakeup_event(ckdev->dev, 0);
> > >  
> > >  		if (ckdev->ec->event_data.event_type == EC_MKBP_EVENT_BUTTON) {
> > 
> > This can also be simplified: `event_data->event_type`.
> 
> Indeed, will update.

Feel free to use my R-b tag after applying the change:
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Re: [PATCH 2/7] Input: cros_ec_keyb - add function key support

[Fabio Baltieri]

On Sat, Feb 21, 2026 at 04:37:10PM -0800, Dmitry Torokhov wrote:
> From: Fabio Baltieri <fabiobaltieri@chromium.org>
> 
> Add support for handling an Fn button and sending separate keycodes for
> a subset of keys in the matrix defined in the upper half of the keymap.
> 
> Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Link: https://patch.msgid.link/20260211173421.1206478-3-fabiobaltieri@chromium.org
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks, happy with the changes, gave it another run on my test setup
with the whole patchset, all looking good.

Thanks,
Fabio