[PATCH v1 0/3] Input: cros_ec_keyb: add function key support

[PATCH v1 0/3] Input: cros_ec_keyb: add function key support

[Fabio Baltieri]

Hi,

this adds function key support to the cros_ec_keyb driver: the platform
can specify a key to be used as "function key", and that changes the
keycode emitted for other keys as long as the function key remains
pressed.

The Fn key omits its own code if pressed and released with no other
function key pressed in the meantime. Non mapped keys do not emit any
codes, this seems to be the behavior of other devices I have lying
around.

Fabio Baltieri (3):
  Input: cros_ec_keyb: clarify key event error message
  Input: cros_ec_keyb: add function key support
  dt-bindings: google,cros-ec-keyb: add fn-key and f-keymap props

 .../bindings/input/google,cros-ec-keyb.yaml   |  60 +++++-
 drivers/input/keyboard/cros_ec_keyb.c         | 193 ++++++++++++++++--
 2 files changed, 227 insertions(+), 26 deletions(-)

-- 
2.52.0.223.gf5cc29aaa4-goog

[PATCH v1 1/3] Input: cros_ec_keyb: clarify key event error message

[Fabio Baltieri]

Reword one of the key event error messages to clarify its meaning: it's
not necessarily an incomplete message, more of a mismatch length.
Clarify that and log the expected and received length too.

Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
---
 drivers/input/keyboard/cros_ec_keyb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 1c6b0461dc35..2822c592880b 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -269,7 +269,8 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
 
 		if (ckdev->ec->event_size != ckdev->cols) {
 			dev_err(ckdev->dev,
-				"Discarded incomplete key matrix event.\n");
+				"Discarded key matrix event, unexpected length: %d != %d\n",
+				ckdev->ec->event_size, ckdev->cols);
 			return NOTIFY_OK;
 		}
 
-- 
2.52.0.223.gf5cc29aaa4-goog

Re: [PATCH v1 1/3] Input: cros_ec_keyb: clarify key event error message

[Simon Glass]

On Tue, 9 Dec 2025 at 08:47, Fabio Baltieri <fabiobaltieri@chromium.org> wrote:
>
> Reword one of the key event error messages to clarify its meaning: it's
> not necessarily an incomplete message, more of a mismatch length.
> Clarify that and log the expected and received length too.
>
> Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> ---
>  drivers/input/keyboard/cros_ec_keyb.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <simon.glass@canonical.com>


>
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index 1c6b0461dc35..2822c592880b 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -269,7 +269,8 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
>
>                 if (ckdev->ec->event_size != ckdev->cols) {
>                         dev_err(ckdev->dev,
> -                               "Discarded incomplete key matrix event.\n");
> +                               "Discarded key matrix event, unexpected length: %d != %d\n",
> +                               ckdev->ec->event_size, ckdev->cols);
>                         return NOTIFY_OK;
>                 }
>
> --
> 2.52.0.223.gf5cc29aaa4-goog
>

Re: [PATCH v1 1/3] Input: cros_ec_keyb: clarify key event error message

[Dmitry Torokhov]

On Tue, Dec 09, 2025 at 03:47:04PM +0000, Fabio Baltieri wrote:
> Reword one of the key event error messages to clarify its meaning: it's
> not necessarily an incomplete message, more of a mismatch length.
> Clarify that and log the expected and received length too.
> 
> Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>

Applied, thank you.

-- 
Dmitry

[PATCH v1 2/3] Input: cros_ec_keyb: add function key support

[Fabio Baltieri]

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

Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
---
 drivers/input/keyboard/cros_ec_keyb.c | 190 ++++++++++++++++++++++++--
 1 file changed, 176 insertions(+), 14 deletions(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 2822c592880b..b0965e5d20de 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -29,6 +29,14 @@
 
 #include <linux/unaligned.h>
 
+/* Maximum number of Fn keys, limited by the key status mask size. */
+#define CROS_EC_FN_KEYMAP_MAX 32
+
+/* 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 +52,13 @@
  * @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
+ * @fn_key: coordinate of the function key
+ * @fn_keymap: array of coordinate and codes for the function keys
+ * @fn_keymap_len: number of entries in the fn_keymap array
+ * @fn_key_status: active function keys bitmap
+ * @normal_key_status: active normal keys bitmap
+ * @fn_key_pressed: tracks the function key status
+ * @fn_key_triggered: tracks where any function key fired
  */
 struct cros_ec_keyb {
 	unsigned int rows;
@@ -61,6 +76,14 @@ struct cros_ec_keyb {
 	struct notifier_block notifier;
 
 	struct vivaldi_data vdata;
+
+	uint32_t fn_key;
+	uint32_t *fn_keymap;
+	int fn_keymap_len;
+	uint32_t fn_key_status;
+	uint8_t normal_key_status[CROS_EC_KEYBOARD_COLS_MAX];
+	bool fn_key_pressed;
+	bool fn_key_triggered;
 };
 
 /**
@@ -166,16 +189,108 @@ static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
 	return false;
 }
 
+static bool cros_ec_key_is(int row, int col, uint32_t key)
+{
+	if (row == KEY_ROW(key) && col == KEY_COL(key))
+		return true;
+
+	return false;
+}
+
+static void cros_ec_keyb_process_one(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);
+	unsigned int code = keycodes[pos];
+
+	dev_dbg(ckdev->dev, "changed: [r%d c%d]: byte %02x\n", row, col, state);
+
+	if (ckdev->fn_keymap) {
+		if (cros_ec_key_is(row, col, ckdev->fn_key)) {
+			ckdev->fn_key_pressed = state;
+
+			if (state) {
+				ckdev->fn_key_triggered = false;
+			} else if (!ckdev->fn_key_triggered) {
+				/*
+				 * Send the original code if nothing else has
+				 * been pressed together with Fn.
+				 */
+				input_event(idev, EV_MSC, MSC_SCAN, pos);
+				input_report_key(idev, code, true);
+				input_sync(ckdev->idev);
+
+				input_event(idev, EV_MSC, MSC_SCAN, pos);
+				input_report_key(idev, code, false);
+			}
+
+			return;
+		}
+
+		if (!state) {
+			/* Key release, may need to release the Fn code */
+			for (int i = 0; i < ckdev->fn_keymap_len; i++) {
+				if (!cros_ec_key_is(row, col,
+						    ckdev->fn_keymap[i]))
+					continue;
+
+				if ((ckdev->fn_key_status & BIT(i)) == 0)
+					continue;
+
+				code = KEY_VAL(ckdev->fn_keymap[i]);
+				ckdev->fn_key_status &= ~BIT(i);
+
+				input_event(idev, EV_MSC, MSC_SCAN, pos);
+				input_report_key(idev, code, state);
+
+				return;
+			}
+
+			if ((ckdev->normal_key_status[col] & BIT(row)) == 0)
+				/* Discard, key press code was not sent */
+				return;
+		} else if (ckdev->fn_key_pressed) {
+			/* Key press while holding Fn */
+			ckdev->fn_key_triggered = true;
+
+			for (int i = 0; i < ckdev->fn_keymap_len; i++) {
+				if (!cros_ec_key_is(row, col,
+						    ckdev->fn_keymap[i]))
+					continue;
+
+				code = KEY_VAL(ckdev->fn_keymap[i]);
+				ckdev->fn_key_status |= BIT(i);
+
+				input_event(idev, EV_MSC, MSC_SCAN, pos);
+				input_report_key(idev, code, state);
+
+				return;
+			}
+
+			/* Do not emit a code if the key is not mapped */
+			return;
+		}
+	}
+
+	if (state)
+		ckdev->normal_key_status[col] |= BIT(row);
+	else
+		ckdev->normal_key_status[col] &= ~BIT(row);
+
+	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 +307,13 @@ 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;
+
+			cros_ec_keyb_process_one(ckdev, row, col, new_state);
 		}
 		ckdev->old_kb_state[col] = kb_state[col];
 	}
@@ -604,6 +712,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;
@@ -660,6 +774,47 @@ static int cros_ec_keyb_register_matrix(struct cros_ec_keyb *ckdev)
 	return 0;
 }
 
+static int cros_ec_keyb_register_fn_keys(struct cros_ec_keyb *ckdev)
+{
+	struct device *dev = ckdev->dev;
+	uint32_t fn_key;
+	uint32_t *keymap;
+	int keymap_len;
+	int ret;
+
+	if (!(device_property_present(dev, "fn-key") &&
+	      device_property_present(dev, "fn-keymap")))
+		return 0;
+
+	device_property_read_u32(dev, "fn-key", &fn_key);
+
+	keymap_len = device_property_count_u32(ckdev->dev, "fn-keymap");
+	if (keymap_len > CROS_EC_FN_KEYMAP_MAX) {
+		dev_err(dev, "fn-keymap too large: %d limit=%d",
+			keymap_len, CROS_EC_FN_KEYMAP_MAX);
+		return -EINVAL;
+	}
+
+	keymap = devm_kcalloc(dev, keymap_len, sizeof(*keymap), GFP_KERNEL);
+	if (!keymap)
+		return -ENOMEM;
+
+	ret = device_property_read_u32_array(dev, "fn-keymap", keymap, keymap_len);
+	if (ret) {
+		dev_err(dev, "failed to read fn-keymap property: %d\n", ret);
+		return ret;
+	}
+
+	for (int i = 0; i < keymap_len; i++)
+		__set_bit(KEY_VAL(keymap[i]), ckdev->idev->keybit);
+
+	ckdev->fn_key = fn_key;
+	ckdev->fn_keymap = keymap;
+	ckdev->fn_keymap_len = keymap_len;
+
+	return 0;
+}
+
 static ssize_t function_row_physmap_show(struct device *dev,
 					 struct device_attribute *attr,
 					 char *buf)
@@ -734,6 +889,13 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
 				err);
 			return err;
 		}
+
+		err = cros_ec_keyb_register_fn_keys(ckdev);
+		if (err) {
+			dev_err(dev, "cannot register fn-keys inputs: %d\n",
+				err);
+			return err;
+		}
 	}
 
 	err = cros_ec_keyb_register_bs(ckdev, buttons_switches_only);
-- 
2.52.0.223.gf5cc29aaa4-goog

Re: [PATCH v1 2/3] Input: cros_ec_keyb: add function key support

[Simon Glass]

Hi Fabio!

On Tue, 9 Dec 2025 at 08:47, Fabio Baltieri <fabiobaltieri@chromium.org> wrote:
>
> Add support for handling an Fn button and sending separate keycodes for
> a subset of keys in the matrix.
>
> Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> ---
>  drivers/input/keyboard/cros_ec_keyb.c | 190 ++++++++++++++++++++++++--
>  1 file changed, 176 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index 2822c592880b..b0965e5d20de 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -29,6 +29,14 @@
>
>  #include <linux/unaligned.h>
>
> +/* Maximum number of Fn keys, limited by the key status mask size. */
> +#define CROS_EC_FN_KEYMAP_MAX 32
> +
> +/* 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 +52,13 @@
>   * @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
> + * @fn_key: coordinate of the function key
> + * @fn_keymap: array of coordinate and codes for the function keys
> + * @fn_keymap_len: number of entries in the fn_keymap array
> + * @fn_key_status: active function keys bitmap
> + * @normal_key_status: active normal keys bitmap
> + * @fn_key_pressed: tracks the function key status
> + * @fn_key_triggered: tracks where any function key fired
>   */
>  struct cros_ec_keyb {
>         unsigned int rows;
> @@ -61,6 +76,14 @@ struct cros_ec_keyb {
>         struct notifier_block notifier;
>
>         struct vivaldi_data vdata;
> +
> +       uint32_t fn_key;

Normally we use u32/u8 these days

> +       uint32_t *fn_keymap;
> +       int fn_keymap_len;
> +       uint32_t fn_key_status;
> +       uint8_t normal_key_status[CROS_EC_KEYBOARD_COLS_MAX];
> +       bool fn_key_pressed;
> +       bool fn_key_triggered;
>  };
>
>  /**
> @@ -166,16 +189,108 @@ static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
>         return false;
>  }
>
> +static bool cros_ec_key_is(int row, int col, uint32_t key)
> +{
> +       if (row == KEY_ROW(key) && col == KEY_COL(key))
> +               return true;
> +
> +       return false;
> +}
> +
> +static void cros_ec_keyb_process_one(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);
> +       unsigned int code = keycodes[pos];
> +
> +       dev_dbg(ckdev->dev, "changed: [r%d c%d]: byte %02x\n", row, col, state);
> +
> +       if (ckdev->fn_keymap) {
> +               if (cros_ec_key_is(row, col, ckdev->fn_key)) {
> +                       ckdev->fn_key_pressed = state;
> +
> +                       if (state) {
> +                               ckdev->fn_key_triggered = false;
> +                       } else if (!ckdev->fn_key_triggered) {
> +                               /*
> +                                * Send the original code if nothing else has
> +                                * been pressed together with Fn.
> +                                */
> +                               input_event(idev, EV_MSC, MSC_SCAN, pos);
> +                               input_report_key(idev, code, true);
> +                               input_sync(ckdev->idev);

What is this function? I might be missing a patch?

> +
> +                               input_event(idev, EV_MSC, MSC_SCAN, pos);
> +                               input_report_key(idev, code, false);
> +                       }
> +
> +                       return;
> +               }
> +
> +               if (!state) {
> +                       /* Key release, may need to release the Fn code */
> +                       for (int i = 0; i < ckdev->fn_keymap_len; i++) {
> +                               if (!cros_ec_key_is(row, col,
> +                                                   ckdev->fn_keymap[i]))
> +                                       continue;
> +
> +                               if ((ckdev->fn_key_status & BIT(i)) == 0)
> +                                       continue;
> +
> +                               code = KEY_VAL(ckdev->fn_keymap[i]);
> +                               ckdev->fn_key_status &= ~BIT(i);
> +
> +                               input_event(idev, EV_MSC, MSC_SCAN, pos);
> +                               input_report_key(idev, code, state);
> +
> +                               return;
> +                       }
> +
> +                       if ((ckdev->normal_key_status[col] & BIT(row)) == 0)
> +                               /* Discard, key press code was not sent */
> +                               return;
> +               } else if (ckdev->fn_key_pressed) {
> +                       /* Key press while holding Fn */
> +                       ckdev->fn_key_triggered = true;
> +
> +                       for (int i = 0; i < ckdev->fn_keymap_len; i++) {
> +                               if (!cros_ec_key_is(row, col,
> +                                                   ckdev->fn_keymap[i]))
> +                                       continue;
> +
> +                               code = KEY_VAL(ckdev->fn_keymap[i]);
> +                               ckdev->fn_key_status |= BIT(i);
> +
> +                               input_event(idev, EV_MSC, MSC_SCAN, pos);
> +                               input_report_key(idev, code, state);
> +
> +                               return;
> +                       }
> +
> +                       /* Do not emit a code if the key is not mapped */
> +                       return;
> +               }
> +       }

I think this function could do with splitting a bit

> +
> +       if (state)
> +               ckdev->normal_key_status[col] |= BIT(row);
> +       else
> +               ckdev->normal_key_status[col] &= ~BIT(row);
> +
> +       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 +307,13 @@ 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;
> +
> +                       cros_ec_keyb_process_one(ckdev, row, col, new_state);
>                 }
>                 ckdev->old_kb_state[col] = kb_state[col];
>         }
> @@ -604,6 +712,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;
> @@ -660,6 +774,47 @@ static int cros_ec_keyb_register_matrix(struct cros_ec_keyb *ckdev)
>         return 0;
>  }
>
> +static int cros_ec_keyb_register_fn_keys(struct cros_ec_keyb *ckdev)
> +{
> +       struct device *dev = ckdev->dev;
> +       uint32_t fn_key;
> +       uint32_t *keymap;
> +       int keymap_len;
> +       int ret;
> +
> +       if (!(device_property_present(dev, "fn-key") &&
> +             device_property_present(dev, "fn-keymap")))
> +               return 0;
> +
> +       device_property_read_u32(dev, "fn-key", &fn_key);
> +
> +       keymap_len = device_property_count_u32(ckdev->dev, "fn-keymap");
> +       if (keymap_len > CROS_EC_FN_KEYMAP_MAX) {
> +               dev_err(dev, "fn-keymap too large: %d limit=%d",
> +                       keymap_len, CROS_EC_FN_KEYMAP_MAX);
> +               return -EINVAL;
> +       }
> +
> +       keymap = devm_kcalloc(dev, keymap_len, sizeof(*keymap), GFP_KERNEL);
> +       if (!keymap)
> +               return -ENOMEM;
> +
> +       ret = device_property_read_u32_array(dev, "fn-keymap", keymap, keymap_len);
> +       if (ret) {
> +               dev_err(dev, "failed to read fn-keymap property: %d\n", ret);
> +               return ret;
> +       }
> +
> +       for (int i = 0; i < keymap_len; i++)
> +               __set_bit(KEY_VAL(keymap[i]), ckdev->idev->keybit);
> +
> +       ckdev->fn_key = fn_key;
> +       ckdev->fn_keymap = keymap;
> +       ckdev->fn_keymap_len = keymap_len;
> +
> +       return 0;
> +}
> +
>  static ssize_t function_row_physmap_show(struct device *dev,
>                                          struct device_attribute *attr,
>                                          char *buf)
> @@ -734,6 +889,13 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
>                                 err);
>                         return err;
>                 }
> +
> +               err = cros_ec_keyb_register_fn_keys(ckdev);
> +               if (err) {
> +                       dev_err(dev, "cannot register fn-keys inputs: %d\n",
> +                               err);
> +                       return err;
> +               }
>         }
>
>         err = cros_ec_keyb_register_bs(ckdev, buttons_switches_only);
> --
> 2.52.0.223.gf5cc29aaa4-goog
>

Can the sandbox driver support this too?

Regards,
Simon

Re: [PATCH v1 2/3] Input: cros_ec_keyb: add function key support

[Fabio Baltieri]

Hey Simon,

On Thu, Dec 11, 2025 at 06:29:01AM -0700, Simon Glass wrote:
> > @@ -44,6 +52,13 @@
> >   * @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
> > + * @fn_key: coordinate of the function key
> > + * @fn_keymap: array of coordinate and codes for the function keys
> > + * @fn_keymap_len: number of entries in the fn_keymap array
> > + * @fn_key_status: active function keys bitmap
> > + * @normal_key_status: active normal keys bitmap
> > + * @fn_key_pressed: tracks the function key status
> > + * @fn_key_triggered: tracks where any function key fired
> >   */
> >  struct cros_ec_keyb {
> >         unsigned int rows;
> > @@ -61,6 +76,14 @@ struct cros_ec_keyb {
> >         struct notifier_block notifier;
> >
> >         struct vivaldi_data vdata;
> > +
> > +       uint32_t fn_key;
> 
> Normally we use u32/u8 these days

Okay, I did notice the file was a bit of a mix, I'll change them in v2.

> 
> > +       uint32_t *fn_keymap;
> > +       int fn_keymap_len;
> > +       uint32_t fn_key_status;
> > +       uint8_t normal_key_status[CROS_EC_KEYBOARD_COLS_MAX];
> > +       bool fn_key_pressed;
> > +       bool fn_key_triggered;
> >  };
> >
> >  /**
> > @@ -166,16 +189,108 @@ static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
> >         return false;
> >  }
> >
> > +static bool cros_ec_key_is(int row, int col, uint32_t key)
> > +{
> > +       if (row == KEY_ROW(key) && col == KEY_COL(key))
> > +               return true;
> > +
> > +       return false;
> > +}
> > +
> > +static void cros_ec_keyb_process_one(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);
> > +       unsigned int code = keycodes[pos];
> > +
> > +       dev_dbg(ckdev->dev, "changed: [r%d c%d]: byte %02x\n", row, col, state);
> > +
> > +       if (ckdev->fn_keymap) {
> > +               if (cros_ec_key_is(row, col, ckdev->fn_key)) {
> > +                       ckdev->fn_key_pressed = state;
> > +
> > +                       if (state) {
> > +                               ckdev->fn_key_triggered = false;
> > +                       } else if (!ckdev->fn_key_triggered) {
> > +                               /*
> > +                                * Send the original code if nothing else has
> > +                                * been pressed together with Fn.
> > +                                */
> > +                               input_event(idev, EV_MSC, MSC_SCAN, pos);
> > +                               input_report_key(idev, code, true);
> > +                               input_sync(ckdev->idev);
> 
> What is this function? I might be missing a patch?

input_sync? it sends an EV_SYN, been there from the start, though I
noticed I miss one two lines below, was relying on the rest of the
function to send it but I changed the logic at some point and broke that
path, will fix that.

> 
> > +
> > +                               input_event(idev, EV_MSC, MSC_SCAN, pos);
> > +                               input_report_key(idev, code, false);
> > +                       }
> > +
> > +                       return;
> > +               }
> > +
> > +               if (!state) {
> > +                       /* Key release, may need to release the Fn code */
> > +                       for (int i = 0; i < ckdev->fn_keymap_len; i++) {
> > +                               if (!cros_ec_key_is(row, col,
> > +                                                   ckdev->fn_keymap[i]))
> > +                                       continue;
> > +
> > +                               if ((ckdev->fn_key_status & BIT(i)) == 0)
> > +                                       continue;
> > +
> > +                               code = KEY_VAL(ckdev->fn_keymap[i]);
> > +                               ckdev->fn_key_status &= ~BIT(i);
> > +
> > +                               input_event(idev, EV_MSC, MSC_SCAN, pos);
> > +                               input_report_key(idev, code, state);
> > +
> > +                               return;
> > +                       }
> > +
> > +                       if ((ckdev->normal_key_status[col] & BIT(row)) == 0)
> > +                               /* Discard, key press code was not sent */
> > +                               return;
> > +               } else if (ckdev->fn_key_pressed) {
> > +                       /* Key press while holding Fn */
> > +                       ckdev->fn_key_triggered = true;
> > +
> > +                       for (int i = 0; i < ckdev->fn_keymap_len; i++) {
> > +                               if (!cros_ec_key_is(row, col,
> > +                                                   ckdev->fn_keymap[i]))
> > +                                       continue;
> > +
> > +                               code = KEY_VAL(ckdev->fn_keymap[i]);
> > +                               ckdev->fn_key_status |= BIT(i);
> > +
> > +                               input_event(idev, EV_MSC, MSC_SCAN, pos);
> > +                               input_report_key(idev, code, state);
> > +
> > +                               return;
> > +                       }
> > +
> > +                       /* Do not emit a code if the key is not mapped */
> > +                       return;
> > +               }
> > +       }
> 
> I think this function could do with splitting a bit

Yeah, I don't love it either but there's a lot of logic intertwined in
there, tried to split it myself and ended up breaking stuff, the logic
for the fn key itsel though can go that's a good 16 lines, I'll start
with that, send a v2 and then go from there.

> Can the sandbox driver support this too?

Not sure what you are referring to, can you give me a pointer?

Hey thanks for the review, good to hear from you. :-)

Cheers,
Fabio

-- 
Fabio Baltieri

[PATCH v1 3/3] dt-bindings: google,cros-ec-keyb: add fn-key and f-keymap props

[Fabio Baltieri]

Add binding documentation for the fn-key and fn-keymap properties,
verify that the two new properties are either both preseent or none.

Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
---
 .../bindings/input/google,cros-ec-keyb.yaml   | 60 +++++++++++++++----
 1 file changed, 49 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
index fefaaf46a240..56adf9026010 100644
--- a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
+++ b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
@@ -44,6 +44,20 @@ properties:
       where the lower 16 bits are reserved. This property is specified only
       when the keyboard has a custom design for the top row keys.
 
+  fn-key:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      An u32 containing the coordinate of the Fn key, use the MATRIX_KEY(row,
+      col, code) macro, code is ignored.
+
+  fn-keymap:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 32
+    description: |
+      An array of u32 mapping the row, column and codes for the function keys,
+      use the MATRIX_KEY macro to define the elements.
+
 dependencies:
   function-row-physmap: [ 'linux,keymap' ]
   google,needs-ghost-filter: [ 'linux,keymap' ]
@@ -51,17 +65,28 @@ dependencies:
 required:
   - compatible
 
-if:
-  properties:
-    compatible:
-      contains:
-        const: google,cros-ec-keyb
-then:
-  $ref: /schemas/input/matrix-keymap.yaml#
-  required:
-    - keypad,num-rows
-    - keypad,num-columns
-    - linux,keymap
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: google,cros-ec-keyb
+    then:
+      $ref: /schemas/input/matrix-keymap.yaml#
+      required:
+        - keypad,num-rows
+        - keypad,num-columns
+        - linux,keymap
+  - if:
+      anyOf:
+        - required:
+          - fn-key
+        - required:
+          - fn-keymap
+    then:
+      required:
+        - fn-key
+        - fn-keymap
 
 unevaluatedProperties: false
 
@@ -132,6 +157,19 @@ examples:
             /* UP      LEFT    */
             0x070b0067 0x070c0069>;
     };
+    fn-key = <MATRIX_KEY(0x04, 0x0a, 0)>;
+    fn-keymap = <
+            MATRIX_KEY(0x00, 0x02, KEY_F1)  /* T1 */
+            MATRIX_KEY(0x03, 0x02, KEY_F2)  /* T2 */
+            MATRIX_KEY(0x02, 0x02, KEY_F3)  /* T3 */
+            MATRIX_KEY(0x01, 0x02, KEY_F4)  /* T4 */
+            MATRIX_KEY(0x04, 0x04, KEY_F5)  /* T5 */
+            MATRIX_KEY(0x02, 0x04, KEY_F6)  /* T6 */
+            MATRIX_KEY(0x01, 0x04, KEY_F7)  /* T7 */
+            MATRIX_KEY(0x02, 0x0b, KEY_F8)  /* T8 */
+            MATRIX_KEY(0x01, 0x09, KEY_F9)  /* T9 */
+            MATRIX_KEY(0x00, 0x04, KEY_F10) /* T10 */
+    >;
   - |
     /* No matrix keyboard, just buttons/switches */
     keyboard-controller {
-- 
2.52.0.223.gf5cc29aaa4-goog

Re: [PATCH v1 3/3] dt-bindings: google,cros-ec-keyb: add fn-key and f-keymap props

[Rob Herring (Arm)]

On Tue, 09 Dec 2025 15:47:06 +0000, Fabio Baltieri wrote:
> Add binding documentation for the fn-key and fn-keymap properties,
> verify that the two new properties are either both preseent or none.
> 
> Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> ---
>  .../bindings/input/google,cros-ec-keyb.yaml   | 60 +++++++++++++++----
>  1 file changed, 49 insertions(+), 11 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml:83:11: [warning] wrong indentation: expected 12 but found 10 (indentation)
./Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml:85:11: [warning] wrong indentation: expected 12 but found 10 (indentation)

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/input/google,cros-ec-keyb.example.dts:83.9-89 Properties must precede subnodes
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.dtbs:141: Documentation/devicetree/bindings/input/google,cros-ec-keyb.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1559: dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20251209154706.529784-4-fabiobaltieri@chromium.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v1 3/3] dt-bindings: google,cros-ec-keyb: add fn-key and f-keymap props

[Rob Herring]

On Tue, Dec 09, 2025 at 03:47:06PM +0000, Fabio Baltieri wrote:
> Add binding documentation for the fn-key and fn-keymap properties,
> verify that the two new properties are either both preseent or none.
> 
> Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> ---
>  .../bindings/input/google,cros-ec-keyb.yaml   | 60 +++++++++++++++----
>  1 file changed, 49 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> index fefaaf46a240..56adf9026010 100644
> --- a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> +++ b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> @@ -44,6 +44,20 @@ properties:
>        where the lower 16 bits are reserved. This property is specified only
>        when the keyboard has a custom design for the top row keys.
>  
> +  fn-key:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      An u32 containing the coordinate of the Fn key, use the MATRIX_KEY(row,
> +      col, code) macro, code is ignored.
> +
> +  fn-keymap:

If keymap is linux,keymap, then this should perhaps be linux,fn-keymap. 
Depends if we still think linux,keymap is Linux specific?

> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 32
> +    description: |
> +      An array of u32 mapping the row, column and codes for the function keys,
> +      use the MATRIX_KEY macro to define the elements.
> +
>  dependencies:
>    function-row-physmap: [ 'linux,keymap' ]
>    google,needs-ghost-filter: [ 'linux,keymap' ]
> @@ -51,17 +65,28 @@ dependencies:
>  required:
>    - compatible
>  
> -if:
> -  properties:
> -    compatible:
> -      contains:
> -        const: google,cros-ec-keyb
> -then:
> -  $ref: /schemas/input/matrix-keymap.yaml#
> -  required:
> -    - keypad,num-rows
> -    - keypad,num-columns
> -    - linux,keymap
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: google,cros-ec-keyb
> +    then:
> +      $ref: /schemas/input/matrix-keymap.yaml#
> +      required:
> +        - keypad,num-rows
> +        - keypad,num-columns
> +        - linux,keymap
> +  - if:
> +      anyOf:
> +        - required:
> +          - fn-key
> +        - required:
> +          - fn-keymap
> +    then:
> +      required:
> +        - fn-key
> +        - fn-keymap

This can be more concisely written as:

dependencies:
  fn-key: [fn-keymap]
  fn-keymap: [fn-key]

Re: [PATCH v1 3/3] dt-bindings: google,cros-ec-keyb: add fn-key and f-keymap props

[Fabio Baltieri]

Hey Rob, thanks for the review.

On Tue, Dec 09, 2025 at 01:22:43PM -0600, Rob Herring wrote:
> On Tue, Dec 09, 2025 at 03:47:06PM +0000, Fabio Baltieri wrote:
> > +  fn-key:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +      An u32 containing the coordinate of the Fn key, use the MATRIX_KEY(row,
> > +      col, code) macro, code is ignored.
> > +
> > +  fn-keymap:
> 
> If keymap is linux,keymap, then this should perhaps be linux,fn-keymap. 
> Depends if we still think linux,keymap is Linux specific?

I'm open for suggestions, trying to understand the pattern, these are
specific to this binding I think if anything they should be
google,fn-key and google,fn-keymap, similarly to the existing
google,needs-ghost-filter -- no idea why function-row-physmap was not
prefixed but I guess it slipped in and now it's not worth changing it.

Would it make sense?

Thanks,
Fabio

-- 
Fabio Baltieri

Re: [PATCH v1 3/3] dt-bindings: google,cros-ec-keyb: add fn-key and f-keymap props

[Dmitry Torokhov]

On Wed, Dec 10, 2025 at 06:00:29PM +0000, Fabio Baltieri wrote:
> Hey Rob, thanks for the review.
> 
> On Tue, Dec 09, 2025 at 01:22:43PM -0600, Rob Herring wrote:
> > On Tue, Dec 09, 2025 at 03:47:06PM +0000, Fabio Baltieri wrote:
> > > +  fn-key:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description: |
> > > +      An u32 containing the coordinate of the Fn key, use the MATRIX_KEY(row,
> > > +      col, code) macro, code is ignored.
> > > +
> > > +  fn-keymap:
> > 
> > If keymap is linux,keymap, then this should perhaps be linux,fn-keymap. 
> > Depends if we still think linux,keymap is Linux specific?
> 
> I'm open for suggestions, trying to understand the pattern, these are
> specific to this binding I think if anything they should be
> google,fn-key and google,fn-keymap, similarly to the existing
> google,needs-ghost-filter -- no idea why function-row-physmap was not
> prefixed but I guess it slipped in and now it's not worth changing it.

Just double the number of rows in the regular keymap to accommodate the
FN modifier, no need for separate keymap. Also no need to have fn-key
property, use whatever key that reports KEY_FN. See how it is done in
drivers/input/keyboard/tegra-kbc.c

Thanks.

-- 
Dmitry

Re: [PATCH v1 3/3] dt-bindings: google,cros-ec-keyb: add fn-key and f-keymap props

[Fabio Baltieri]

Hi Dmitry,

On Thu, Dec 11, 2025 at 08:44:02PM -0800, Dmitry Torokhov wrote:
> On Wed, Dec 10, 2025 at 06:00:29PM +0000, Fabio Baltieri wrote:
> > Hey Rob, thanks for the review.
> > 
> > On Tue, Dec 09, 2025 at 01:22:43PM -0600, Rob Herring wrote:
> > > On Tue, Dec 09, 2025 at 03:47:06PM +0000, Fabio Baltieri wrote:
> > > > +  fn-key:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    description: |
> > > > +      An u32 containing the coordinate of the Fn key, use the MATRIX_KEY(row,
> > > > +      col, code) macro, code is ignored.
> > > > +
> > > > +  fn-keymap:
> > > 
> > > If keymap is linux,keymap, then this should perhaps be linux,fn-keymap. 
> > > Depends if we still think linux,keymap is Linux specific?
> > 
> > I'm open for suggestions, trying to understand the pattern, these are
> > specific to this binding I think if anything they should be
> > google,fn-key and google,fn-keymap, similarly to the existing
> > google,needs-ghost-filter -- no idea why function-row-physmap was not
> > prefixed but I guess it slipped in and now it's not worth changing it.
> 
> Just double the number of rows in the regular keymap to accommodate the
> FN modifier, no need for separate keymap. Also no need to have fn-key
> property, use whatever key that reports KEY_FN. See how it is done in
> drivers/input/keyboard/tegra-kbc.c

Had a look at the tegra-kbc driver as you suggested, first thing it
seems like the fn-key functionality there is dead code since 2013,
`use_fn_map` could only be enabled with platform data, not OF, and that
has been removed in 3a495aeada2b, as it stands kbc->use_fn_map can only
be false. I could send a patch to rip off that code if you want me to,
clearly it hasn't been used in a while (unless I'm missing something).

About the extended fn map, I've two problems with it:
- it seems very wasteful: the normal map is loaded in a linear array
  so it can be access directly, which make sense as that's typically
  very densely populated, but in the case of the fn keys that's going to
  be mostly empty, I'd expect ~20 keys top from a 18x8 matrix. So that
  would waste load of space, direct access is good but for ~20 keys I
  think it's fine to scan it, especially since it only happens when Fn
  is pressed.
- I'd end up with two values for cols kicking around the driver, the
  real one and the one used in the map, which I feel adds confusing in
  the code.
- more importantly, one would have to keep the offset in mind when
  setting the keys in dt, we rely on OEM doing this and I think having a
  separate property with a meaningful name and a map with the same
  row,col and different code is more intuitive and would make their life
  easier, especially since we ship with keyboard of different size
  and the offset would be different depending on the device.

As for the fn-key property, unfortunately based on past experience I'd
expect such OEM to want to change that code, I could specify the code
rather than the row,col but I would not plain hardcode. Even my
(thinkpad) laptop sends KEY_WAKEUP for Fn.

Cheers,
Fabio

-- 
Fabio Baltieri

Re: [PATCH v1 3/3] dt-bindings: google,cros-ec-keyb: add fn-key and f-keymap props

[Dmitry Torokhov]

Hi Fabio,

On Tue, Dec 16, 2025 at 12:23:06PM +0000, Fabio Baltieri wrote:
> Hi Dmitry,
> 
> On Thu, Dec 11, 2025 at 08:44:02PM -0800, Dmitry Torokhov wrote:
> > On Wed, Dec 10, 2025 at 06:00:29PM +0000, Fabio Baltieri wrote:
> > > Hey Rob, thanks for the review.
> > > 
> > > On Tue, Dec 09, 2025 at 01:22:43PM -0600, Rob Herring wrote:
> > > > On Tue, Dec 09, 2025 at 03:47:06PM +0000, Fabio Baltieri wrote:
> > > > > +  fn-key:
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > +    description: |
> > > > > +      An u32 containing the coordinate of the Fn key, use the MATRIX_KEY(row,
> > > > > +      col, code) macro, code is ignored.
> > > > > +
> > > > > +  fn-keymap:
> > > > 
> > > > If keymap is linux,keymap, then this should perhaps be linux,fn-keymap. 
> > > > Depends if we still think linux,keymap is Linux specific?
> > > 
> > > I'm open for suggestions, trying to understand the pattern, these are
> > > specific to this binding I think if anything they should be
> > > google,fn-key and google,fn-keymap, similarly to the existing
> > > google,needs-ghost-filter -- no idea why function-row-physmap was not
> > > prefixed but I guess it slipped in and now it's not worth changing it.
> > 
> > Just double the number of rows in the regular keymap to accommodate the
> > FN modifier, no need for separate keymap. Also no need to have fn-key
> > property, use whatever key that reports KEY_FN. See how it is done in
> > drivers/input/keyboard/tegra-kbc.c
> 
> Had a look at the tegra-kbc driver as you suggested, first thing it
> seems like the fn-key functionality there is dead code since 2013,
> `use_fn_map` could only be enabled with platform data, not OF, and that
> has been removed in 3a495aeada2b, as it stands kbc->use_fn_map can only
> be false. I could send a patch to rip off that code if you want me to,
> clearly it hasn't been used in a while (unless I'm missing something).

I guess you are right, we shoudl clean that up. We have another newer
driver that uses the same approach:

drivers/input/keyboard/pinephone-keyboard.c


> About the extended fn map, I've two problems with it:
> - it seems very wasteful: the normal map is loaded in a linear array
>   so it can be access directly, which make sense as that's typically
>   very densely populated, but in the case of the fn keys that's going to
>   be mostly empty, I'd expect ~20 keys top from a 18x8 matrix. So that
>   would waste load of space, direct access is good but for ~20 keys I
>   think it's fine to scan it, especially since it only happens when Fn
>   is pressed.

I am not concerned with this, as this is a singleton device. You
probably "waste" as much space in the code segment by implementing the
custom scanning logic. Additionally with the consolidated keymap
approach you are not breaking ioctls dealing with setting and retrieving
key codes.

> - I'd end up with two values for cols kicking around the driver, the
>   real one and the one used in the map, which I feel adds confusing in
>   the code.

Not sure I follow. You still have the same row and col reported, just
when figuring out the final keycode you need to add an offset.

> - more importantly, one would have to keep the offset in mind when
>   setting the keys in dt, we rely on OEM doing this and I think having a

Do we now? I thought we retain greater control over this. Maybe we
should sync internally.

>   separate property with a meaningful name and a map with the same
>   row,col and different code is more intuitive and would make their life
>   easier, especially since we ship with keyboard of different size
>   and the offset would be different depending on the device.
> 
> As for the fn-key property, unfortunately based on past experience I'd
> expect such OEM to want to change that code, I could specify the code
> rather than the row,col but I would not plain hardcode. Even my
> (thinkpad) laptop sends KEY_WAKEUP for Fn.

Again, we need to make sure we control OEMs better. On Lenovo Fn sends
wakeup only if it is not combined with another key, so it really has
custom logic with events delivered either through the main AT keyboard
or through custom interface in thinkpad platform driver. We do not need
this in oiur designs.

Thanks.

-- 
Dmitry

Re: [PATCH v1 3/3] dt-bindings: google,cros-ec-keyb: add fn-key and f-keymap props

[Fabio Baltieri]

On Wed, Dec 17, 2025 at 10:05:45AM -0800, Dmitry Torokhov wrote:
> > Had a look at the tegra-kbc driver as you suggested, first thing it
> > seems like the fn-key functionality there is dead code since 2013,
> > `use_fn_map` could only be enabled with platform data, not OF, and that
> > has been removed in 3a495aeada2b, as it stands kbc->use_fn_map can only
> > be false. I could send a patch to rip off that code if you want me to,
> > clearly it hasn't been used in a while (unless I'm missing something).
> 
> I guess you are right, we shoudl clean that up. We have another newer
> driver that uses the same approach:
> 
> drivers/input/keyboard/pinephone-keyboard.c

Alright I'll look into it and rework a v2 with the extended map, checked
the code again and I see your arguments, sounds reasonable.

Cheers,
Fabio

-- 
Fabio Baltieri

Re: [PATCH v1 3/3] dt-bindings: google,cros-ec-keyb: add fn-key and f-keymap props

[kernel test robot]

:::::: 
:::::: Manual check reason: "dtcheck: binding changes may go via different trees"
:::::: 

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20251209154706.529784-4-fabiobaltieri@chromium.org>
References: <20251209154706.529784-4-fabiobaltieri@chromium.org>
TO: Fabio Baltieri <fabiobaltieri@chromium.org>
TO: Dmitry Torokhov <dmitry.torokhov@gmail.com>
TO: Rob Herring <robh@kernel.org>
TO: Krzysztof Kozlowski <krzk@kernel.org>
TO: Conor Dooley <conor+dt@kernel.org>
TO: Benson Leung <bleung@chromium.org>
TO: Guenter Roeck <groeck@chromium.org>
CC: Fabio Baltieri <fabiobaltieri@chromium.org>
CC: "Tzung-Bi Shih" <tzungbi@kernel.org>
CC: Simon Glass <sjg@chromium.org>
CC: linux-input@vger.kernel.org
CC: devicetree@vger.kernel.org
CC: chrome-platform@lists.linux.dev
CC: linux-kernel@vger.kernel.orga

Hi Fabio,

kernel test robot noticed the following build errors:

[auto build test ERROR on dtor-input/next]
[also build test ERROR on dtor-input/for-linus robh/for-next linus/master v6.16-rc1]
[cannot apply to next-20251215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Fabio-Baltieri/Input-cros_ec_keyb-clarify-key-event-error-message/20251209-235043
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link:    https://lore.kernel.org/r/20251209154706.529784-4-fabiobaltieri%40chromium.org
patch subject: [PATCH v1 3/3] dt-bindings: google,cros-ec-keyb: add fn-key and f-keymap props
:::::: branch date: 6 days ago
:::::: commit date: 6 days ago
config: arm64-randconfig-2052-20251214 (https://download.01.org/0day-ci/archive/20251216/202512160313.xQWscEOe-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project ecaf673850beb241957352bd61e95ed34256635f)
dtschema version: 2025.12
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251216/202512160313.xQWscEOe-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/r/202512160313.xQWscEOe-lkp@intel.com/

All errors (new ones prefixed by >>):

   Error: Documentation/devicetree/bindings/input/google,cros-ec-keyb.example.dts:83.9-89 Properties must precede subnodes
>> FATAL ERROR: Unable to parse input tree

dtcheck warnings: (new ones prefixed by >>)
>> Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml:83:11: [warning] wrong indentation: expected 12 but found 10 (indentation)
   Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml:85:11: [warning] wrong indentation: expected 12 but found 10 (indentation)

vim +83 Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml

50d68feee01039 Ricardo Cañuelo 2020-10-21   9  
50d68feee01039 Ricardo Cañuelo 2020-10-21  10  maintainers:
50d68feee01039 Ricardo Cañuelo 2020-10-21  11    - Simon Glass <sjg@chromium.org>
50d68feee01039 Ricardo Cañuelo 2020-10-21  12    - Benson Leung <bleung@chromium.org>
50d68feee01039 Ricardo Cañuelo 2020-10-21  13  
50d68feee01039 Ricardo Cañuelo 2020-10-21  14  description: |
50d68feee01039 Ricardo Cañuelo 2020-10-21  15    Google's ChromeOS EC Keyboard is a simple matrix keyboard
50d68feee01039 Ricardo Cañuelo 2020-10-21  16    implemented on a separate EC (Embedded Controller) device. It provides
50d68feee01039 Ricardo Cañuelo 2020-10-21  17    a message for reading key scans from the EC. These are then converted
52dc6d3bea3b85 Stephen Boyd    2022-05-16  18    into keycodes for processing by the kernel. This device also supports
52dc6d3bea3b85 Stephen Boyd    2022-05-16  19    switches/buttons like power and volume buttons.
50d68feee01039 Ricardo Cañuelo 2020-10-21  20  
50d68feee01039 Ricardo Cañuelo 2020-10-21  21  properties:
50d68feee01039 Ricardo Cañuelo 2020-10-21  22    compatible:
d95bca4fbde0a2 Stephen Boyd    2022-05-18  23      oneOf:
52dc6d3bea3b85 Stephen Boyd    2022-05-16  24        - description: ChromeOS EC with only buttons/switches
d95bca4fbde0a2 Stephen Boyd    2022-05-18  25          const: google,cros-ec-keyb-switches
52dc6d3bea3b85 Stephen Boyd    2022-05-16  26        - description: ChromeOS EC with keyboard and possibly buttons/switches
50d68feee01039 Ricardo Cañuelo 2020-10-21  27          const: google,cros-ec-keyb
50d68feee01039 Ricardo Cañuelo 2020-10-21  28  
50d68feee01039 Ricardo Cañuelo 2020-10-21  29    google,needs-ghost-filter:
50d68feee01039 Ricardo Cañuelo 2020-10-21  30      description:
50d68feee01039 Ricardo Cañuelo 2020-10-21  31        Enable a ghost filter for the matrix keyboard. This is recommended
50d68feee01039 Ricardo Cañuelo 2020-10-21  32        if the EC does not have its own logic or hardware for this.
50d68feee01039 Ricardo Cañuelo 2020-10-21  33      type: boolean
50d68feee01039 Ricardo Cañuelo 2020-10-21  34  
311a27da627139 Philip Chen     2021-02-22  35    function-row-physmap:
4e71ed985389ce Rob Herring     2022-05-19  36      $ref: /schemas/types.yaml#/definitions/uint32-array
311a27da627139 Philip Chen     2021-02-22  37      minItems: 1
311a27da627139 Philip Chen     2021-02-22  38      maxItems: 15
311a27da627139 Philip Chen     2021-02-22  39      description: |
311a27da627139 Philip Chen     2021-02-22  40        An ordered u32 array describing the rows/columns (in the scan matrix)
311a27da627139 Philip Chen     2021-02-22  41        of top row keys from physical left (KEY_F1) to right. Each entry
311a27da627139 Philip Chen     2021-02-22  42        encodes the row/column as:
311a27da627139 Philip Chen     2021-02-22  43        (((row) & 0xFF) << 24) | (((column) & 0xFF) << 16)
311a27da627139 Philip Chen     2021-02-22  44        where the lower 16 bits are reserved. This property is specified only
311a27da627139 Philip Chen     2021-02-22  45        when the keyboard has a custom design for the top row keys.
311a27da627139 Philip Chen     2021-02-22  46  
70fff14a38ad73 Fabio Baltieri  2025-12-09  47    fn-key:
70fff14a38ad73 Fabio Baltieri  2025-12-09  48      $ref: /schemas/types.yaml#/definitions/uint32
70fff14a38ad73 Fabio Baltieri  2025-12-09  49      description: |
70fff14a38ad73 Fabio Baltieri  2025-12-09  50        An u32 containing the coordinate of the Fn key, use the MATRIX_KEY(row,
70fff14a38ad73 Fabio Baltieri  2025-12-09  51        col, code) macro, code is ignored.
70fff14a38ad73 Fabio Baltieri  2025-12-09  52  
70fff14a38ad73 Fabio Baltieri  2025-12-09  53    fn-keymap:
70fff14a38ad73 Fabio Baltieri  2025-12-09  54      $ref: /schemas/types.yaml#/definitions/uint32-array
70fff14a38ad73 Fabio Baltieri  2025-12-09  55      minItems: 1
70fff14a38ad73 Fabio Baltieri  2025-12-09  56      maxItems: 32
70fff14a38ad73 Fabio Baltieri  2025-12-09  57      description: |
70fff14a38ad73 Fabio Baltieri  2025-12-09  58        An array of u32 mapping the row, column and codes for the function keys,
70fff14a38ad73 Fabio Baltieri  2025-12-09  59        use the MATRIX_KEY macro to define the elements.
70fff14a38ad73 Fabio Baltieri  2025-12-09  60  
52dc6d3bea3b85 Stephen Boyd    2022-05-16  61  dependencies:
c6f3b684c2c4e8 Linus Walleij   2023-02-21  62    function-row-physmap: [ 'linux,keymap' ]
52dc6d3bea3b85 Stephen Boyd    2022-05-16  63    google,needs-ghost-filter: [ 'linux,keymap' ]
52dc6d3bea3b85 Stephen Boyd    2022-05-16  64  
50d68feee01039 Ricardo Cañuelo 2020-10-21  65  required:
50d68feee01039 Ricardo Cañuelo 2020-10-21  66    - compatible
50d68feee01039 Ricardo Cañuelo 2020-10-21  67  
70fff14a38ad73 Fabio Baltieri  2025-12-09  68  allOf:
70fff14a38ad73 Fabio Baltieri  2025-12-09  69    - if:
52dc6d3bea3b85 Stephen Boyd    2022-05-16  70        properties:
52dc6d3bea3b85 Stephen Boyd    2022-05-16  71          compatible:
52dc6d3bea3b85 Stephen Boyd    2022-05-16  72            contains:
52dc6d3bea3b85 Stephen Boyd    2022-05-16  73              const: google,cros-ec-keyb
52dc6d3bea3b85 Stephen Boyd    2022-05-16  74      then:
77987b872fcfea Rob Herring     2023-03-21  75        $ref: /schemas/input/matrix-keymap.yaml#
52dc6d3bea3b85 Stephen Boyd    2022-05-16  76        required:
52dc6d3bea3b85 Stephen Boyd    2022-05-16  77          - keypad,num-rows
52dc6d3bea3b85 Stephen Boyd    2022-05-16  78          - keypad,num-columns
52dc6d3bea3b85 Stephen Boyd    2022-05-16  79          - linux,keymap
70fff14a38ad73 Fabio Baltieri  2025-12-09  80    - if:
70fff14a38ad73 Fabio Baltieri  2025-12-09  81        anyOf:
70fff14a38ad73 Fabio Baltieri  2025-12-09  82          - required:
70fff14a38ad73 Fabio Baltieri  2025-12-09 @83            - fn-key
70fff14a38ad73 Fabio Baltieri  2025-12-09  84          - required:
70fff14a38ad73 Fabio Baltieri  2025-12-09  85            - fn-keymap
70fff14a38ad73 Fabio Baltieri  2025-12-09  86      then:
70fff14a38ad73 Fabio Baltieri  2025-12-09  87        required:
70fff14a38ad73 Fabio Baltieri  2025-12-09  88          - fn-key
70fff14a38ad73 Fabio Baltieri  2025-12-09  89          - fn-keymap
52dc6d3bea3b85 Stephen Boyd    2022-05-16  90  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki