| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAFLszThHmN-eGMwwgUhSFbWcbuOYYs-eFh6d6ZVTXekRGv6Hdg@mail.gmail.com>
Date: Sat, 27 Dec 2025 07:24:33 -0700
From: Simon Glass <sjg@...omium.org>
To: Fabio Baltieri <fabiobaltieri@...omium.org>
Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Benson Leung <bleung@...omium.org>, Guenter Roeck <groeck@...omium.org>,
Tzung-Bi Shih <tzungbi@...nel.org>, linux-input@...r.kernel.org,
devicetree@...r.kernel.org, chrome-platform@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] Input: cros_ec_keyb - add function key support
Hi Fabio,
On Wed, 24 Dec 2025 at 08:22, Fabio Baltieri <fabiobaltieri@...omium.org> wrote:
>
> 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@...omium.org>
> ---
> drivers/input/keyboard/cros_ec_keyb.c | 120 ++++++++++++++++++++++----
> 1 file changed, 104 insertions(+), 16 deletions(-)
>
Reviewed-by: Simon Glass <sjg@...omium.org>
I suggest a function comment for the two new functions you add.
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index 1c6b0461dc35..8b95b4f8a37d 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -29,6 +29,11 @@
>
> #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 +49,11 @@
> * @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
> + * @use_fn_overlay: whether the driver use an fn function overlay
> + * @normal_key_status: active normal keys map
> + * @fn_key_status: active function keys map
> + * @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 +71,12 @@ struct cros_ec_keyb {
> struct notifier_block notifier;
>
> struct vivaldi_data vdata;
> +
> + bool use_fn_overlay;
> + u8 normal_key_status[CROS_EC_KEYBOARD_COLS_MAX];
> + u8 fn_key_status[CROS_EC_KEYBOARD_COLS_MAX];
> + bool fn_key_pressed;
> + bool fn_key_triggered;
> };
>
> /**
> @@ -166,16 +182,83 @@ static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
> return false;
> }
>
> +static void cros_ec_keyb_process_fn_key(struct cros_ec_keyb *ckdev,
> + int row, int col, bool state)
> +{
> + struct input_dev *idev = ckdev->idev;
> + int pos = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
> +
> + 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, KEY_FN, true);
> + input_sync(idev);
> +
> + input_event(idev, EV_MSC, MSC_SCAN, pos);
> + input_report_key(idev, KEY_FN, 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->use_fn_overlay) {
> + if (code == KEY_FN)
> + return cros_ec_keyb_process_fn_key(ckdev, row, col, state);
> +
> + if (!state) {
> + if (ckdev->fn_key_status[col] & BIT(row)) {
> + pos = MATRIX_SCAN_CODE(row + ckdev->rows, col, ckdev->row_shift);
> + code = keycodes[pos];
You might want a helper to do this as it is repeated below
> +
> + ckdev->fn_key_status[col] &= ~BIT(row);
> + } else if (ckdev->normal_key_status[col] & BIT(row)) {
> + ckdev->normal_key_status[col] &= ~BIT(row);
> + } else {
> + /* Discard, key press code was not sent */
> + return;
> + }
> + } else if (ckdev->fn_key_pressed) {
> + pos = MATRIX_SCAN_CODE(row + ckdev->rows, col, ckdev->row_shift);
> + code = keycodes[pos];
> +
> + ckdev->fn_key_triggered = true;
> +
> + if (!code)
> + return;
> +
> + ckdev->fn_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 +275,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];
> }
> @@ -597,12 +673,19 @@ static int cros_ec_keyb_register_matrix(struct cros_ec_keyb *ckdev)
> struct device *dev = ckdev->dev;
> struct input_dev *idev;
> const char *phys;
> + unsigned int rows_keymap;
> int err;
>
> err = matrix_keypad_parse_properties(dev, &ckdev->rows, &ckdev->cols);
> 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;
> @@ -635,7 +718,12 @@ static int cros_ec_keyb_register_matrix(struct cros_ec_keyb *ckdev)
> ckdev->ghost_filter = device_property_read_bool(dev,
> "google,needs-ghost-filter");
>
> - err = matrix_keypad_build_keymap(NULL, NULL, ckdev->rows, ckdev->cols,
> + ckdev->use_fn_overlay = device_property_read_bool(dev,
> + "google,use-fn-overlay");
> +
> + rows_keymap = ckdev->use_fn_overlay ? ckdev->rows * 2 : ckdev->rows;
> +
> + err = matrix_keypad_build_keymap(NULL, NULL, rows_keymap, ckdev->cols,
> NULL, idev);
> if (err) {
> dev_err(dev, "cannot build key matrix\n");
> --
> 2.52.0.351.gbe84eed79e-goog
>
Regards,
Simon
Powered by blists - more mailing lists