| 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: <YhyBYl0DbizOwClS@google.com>
Date: Mon, 28 Feb 2022 00:01:38 -0800
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Stephen Boyd <swboyd@...omium.org>
Cc: linux-kernel@...r.kernel.org, benjamin.tissoires@...hat.com,
Jiri Kosina <jikos@...nel.org>, linux-input@...r.kernel.org,
Sean O'Brien <seobrien@...omium.org>,
Douglas Anderson <dianders@...omium.org>,
Zhengqiao Xia <xiazhengqiao@...qin.corp-partner.google.com>,
Jiri Kosina <jkosina@...e.cz>
Subject: Re: [PATCH v4 2/4] HID: Extract vivaldi hid feature mapping for use
in hid-hammer
On Wed, Feb 16, 2022 at 11:58:59AM -0800, Stephen Boyd wrote:
> We need to support parsing the HID device in both the vivaldi and the
> hammer drivers so that we can properly expose the function row physmap
> to userspace when a hammer device uses a vivaldi keyboard layout for the
> function row keys. Extract the feature mapping logic from the vivaldi
> driver into an hid specific vivaldi library so we can use it from both
> HID drivers.
>
> Cc: Jiri Kosina <jikos@...nel.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>
> Tested-by: "Sean O'Brien" <seobrien@...omium.org>
> Cc: Douglas Anderson <dianders@...omium.org>
> Cc: Zhengqiao Xia <xiazhengqiao@...qin.corp-partner.google.com>
> Acked-by: Jiri Kosina <jkosina@...e.cz>
> Signed-off-by: Stephen Boyd <swboyd@...omium.org>
> ---
> drivers/hid/Kconfig | 9 +++
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-vivaldi-common.c | 97 ++++++++++++++++++++++++++++++
> drivers/hid/hid-vivaldi.c | 69 +--------------------
> include/linux/input/vivaldi-fmap.h | 9 +++
> 5 files changed, 118 insertions(+), 67 deletions(-)
> create mode 100644 drivers/hid/hid-vivaldi-common.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 5569a2029dab..ea8fa71c9e9c 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -403,14 +403,23 @@ config HOLTEK_FF
> Say Y here if you have a Holtek On Line Grip based game controller
> and want to have force feedback support for it.
>
> +config HID_VIVALDI_COMMON
> + tristate
> + help
> + ChromeOS Vivaldi HID parsing support library. This is a hidden
> + option so that drivers can use common code to parse the HID
> + descriptors for vivaldi function row keymap.
> +
> config HID_GOOGLE_HAMMER
> tristate "Google Hammer Keyboard"
> + select HID_VIVALDI_COMMON
This chunk belongs to the next patch.
> depends on USB_HID && LEDS_CLASS && CROS_EC
> help
> Say Y here if you have a Google Hammer device.
>
> config HID_VIVALDI
> tristate "Vivaldi Keyboard"
> + select HID_VIVALDI_COMMON
> select INPUT_VIVALDIFMAP
> depends on HID
> help
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 6d3e630e81af..469a6159ebae 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -50,6 +50,7 @@ obj-$(CONFIG_HID_FT260) += hid-ft260.o
> obj-$(CONFIG_HID_GEMBIRD) += hid-gembird.o
> obj-$(CONFIG_HID_GFRM) += hid-gfrm.o
> obj-$(CONFIG_HID_GLORIOUS) += hid-glorious.o
> +obj-$(CONFIG_HID_VIVALDI_COMMON) += hid-vivaldi-common.o
> obj-$(CONFIG_HID_GOOGLE_HAMMER) += hid-google-hammer.o
> obj-$(CONFIG_HID_VIVALDI) += hid-vivaldi.o
> obj-$(CONFIG_HID_GT683R) += hid-gt683r.o
> diff --git a/drivers/hid/hid-vivaldi-common.c b/drivers/hid/hid-vivaldi-common.c
> new file mode 100644
> index 000000000000..8a5074fd63b7
> --- /dev/null
> +++ b/drivers/hid/hid-vivaldi-common.c
> @@ -0,0 +1,97 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Helpers for ChromeOS HID Vivaldi keyboards
> + *
> + * Copyright (C) 2022 Google, Inc
> + */
> +
> +#include <linux/export.h>
> +#include <linux/hid.h>
> +#include <linux/input/vivaldi-fmap.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/types.h>
> +
> +#define HID_VD_FN_ROW_PHYSMAP 0x00000001
> +#define HID_USAGE_FN_ROW_PHYSMAP (HID_UP_GOOGLEVENDOR | HID_VD_FN_ROW_PHYSMAP)
> +
> +/**
> + * vivaldi_hid_feature_mapping - Fill out vivaldi keymap data exposed via HID
> + * @data: The vivaldi function keymap
> + * @hdev: HID device to parse
> + * @field: HID field to parse
> + * @usage: HID usage to parse
> + */
> +void vivaldi_hid_feature_mapping(struct vivaldi_data *data,
> + struct hid_device *hdev,
> + struct hid_field *field,
> + struct hid_usage *usage)
> +{
> + struct hid_report *report = field->report;
> + int fn_key;
> + int ret;
> + u32 report_len;
> + u8 *report_data, *buf;
> +
> + if (field->logical != HID_USAGE_FN_ROW_PHYSMAP ||
> + (usage->hid & HID_USAGE_PAGE) != HID_UP_ORDINAL)
> + return;
> +
> + fn_key = (usage->hid & HID_USAGE);
> + if (fn_key < VIVALDI_MIN_FN_ROW_KEY || fn_key > VIVALDI_MAX_FN_ROW_KEY)
> + return;
> + if (fn_key > data->num_function_row_keys)
> + data->num_function_row_keys = fn_key;
> +
> + report_data = buf = hid_alloc_report_buf(report, GFP_KERNEL);
> + if (!report_data)
> + return;
> +
> + report_len = hid_report_len(report);
> + if (!report->id) {
> + /*
> + * hid_hw_raw_request() will stuff report ID (which will be 0)
> + * into the first byte of the buffer even for unnumbered
> + * reports, so we need to account for this to avoid getting
> + * -EOVERFLOW in return.
> + * Note that hid_alloc_report_buf() adds 7 bytes to the size
> + * so we can safely say that we have space for an extra byte.
> + */
> + report_len++;
> + }
> +
> + ret = hid_hw_raw_request(hdev, report->id, report_data,
> + report_len, HID_FEATURE_REPORT,
> + HID_REQ_GET_REPORT);
> + if (ret < 0) {
> + dev_warn(&hdev->dev, "failed to fetch feature %d\n",
> + field->report->id);
> + goto out;
> + }
> +
> + if (!report->id) {
> + /*
> + * Undo the damage from hid_hw_raw_request() for unnumbered
> + * reports.
> + */
> + report_data++;
> + report_len--;
> + }
> +
> + ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, report_data,
> + report_len, 0);
> + if (ret) {
> + dev_warn(&hdev->dev, "failed to report feature %d\n",
> + field->report->id);
> + goto out;
> + }
> +
> + data->function_row_physmap[fn_key - VIVALDI_MIN_FN_ROW_KEY] =
> + field->value[usage->usage_index];
> +
> +out:
> + kfree(buf);
> +}
> +EXPORT_SYMBOL_GPL(vivaldi_hid_feature_mapping);
> +
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hid/hid-vivaldi.c b/drivers/hid/hid-vivaldi.c
> index adb56b342948..f70cab6a192b 100644
> --- a/drivers/hid/hid-vivaldi.c
> +++ b/drivers/hid/hid-vivaldi.c
> @@ -13,9 +13,6 @@
> #include <linux/module.h>
> #include <linux/sysfs.h>
>
> -#define HID_VD_FN_ROW_PHYSMAP 0x00000001
> -#define HID_USAGE_FN_ROW_PHYSMAP (HID_UP_GOOGLEVENDOR | HID_VD_FN_ROW_PHYSMAP)
> -
> static ssize_t function_row_physmap_show(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> @@ -60,70 +57,8 @@ static void vivaldi_feature_mapping(struct hid_device *hdev,
> struct hid_usage *usage)
> {
> struct vivaldi_data *drvdata = hid_get_drvdata(hdev);
> - struct hid_report *report = field->report;
> - int fn_key;
> - int ret;
> - u32 report_len;
> - u8 *report_data, *buf;
> -
> - if (field->logical != HID_USAGE_FN_ROW_PHYSMAP ||
> - (usage->hid & HID_USAGE_PAGE) != HID_UP_ORDINAL)
> - return;
> -
> - fn_key = (usage->hid & HID_USAGE);
> - if (fn_key < VIVALDI_MIN_FN_ROW_KEY || fn_key > VIVALDI_MAX_FN_ROW_KEY)
> - return;
> - if (fn_key > drvdata->num_function_row_keys)
> - drvdata->num_function_row_keys = fn_key;
> -
> - report_data = buf = hid_alloc_report_buf(report, GFP_KERNEL);
> - if (!report_data)
> - return;
> -
> - report_len = hid_report_len(report);
> - if (!report->id) {
> - /*
> - * hid_hw_raw_request() will stuff report ID (which will be 0)
> - * into the first byte of the buffer even for unnumbered
> - * reports, so we need to account for this to avoid getting
> - * -EOVERFLOW in return.
> - * Note that hid_alloc_report_buf() adds 7 bytes to the size
> - * so we can safely say that we have space for an extra byte.
> - */
> - report_len++;
> - }
> -
> - ret = hid_hw_raw_request(hdev, report->id, report_data,
> - report_len, HID_FEATURE_REPORT,
> - HID_REQ_GET_REPORT);
> - if (ret < 0) {
> - dev_warn(&hdev->dev, "failed to fetch feature %d\n",
> - field->report->id);
> - goto out;
> - }
> -
> - if (!report->id) {
> - /*
> - * Undo the damage from hid_hw_raw_request() for unnumbered
> - * reports.
> - */
> - report_data++;
> - report_len--;
> - }
> -
> - ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, report_data,
> - report_len, 0);
> - if (ret) {
> - dev_warn(&hdev->dev, "failed to report feature %d\n",
> - field->report->id);
> - goto out;
> - }
> -
> - drvdata->function_row_physmap[fn_key - VIVALDI_MIN_FN_ROW_KEY] =
> - field->value[usage->usage_index];
> -
> -out:
> - kfree(buf);
> +
> + vivaldi_hid_feature_mapping(drvdata, hdev, field, usage);
> }
>
> static int vivaldi_input_configured(struct hid_device *hdev,
> diff --git a/include/linux/input/vivaldi-fmap.h b/include/linux/input/vivaldi-fmap.h
> index 57563d9da022..c736200b4511 100644
> --- a/include/linux/input/vivaldi-fmap.h
> +++ b/include/linux/input/vivaldi-fmap.h
> @@ -4,6 +4,10 @@
>
> #include <linux/types.h>
>
> +struct hid_device;
> +struct hid_field;
> +struct hid_usage;
> +
This all HID-specific and does not belong here, I created a new
hid-vivaldi-common.h in drivers/hid for it.
> #define VIVALDI_MIN_FN_ROW_KEY 1
> #define VIVALDI_MAX_FN_ROW_KEY 24
>
> @@ -25,4 +29,9 @@ struct vivaldi_data {
> ssize_t vivaldi_function_row_physmap_show(const struct vivaldi_data *data,
> char *buf);
>
> +void vivaldi_hid_feature_mapping(struct vivaldi_data *data,
> + struct hid_device *hdev,
> + struct hid_field *field,
> + struct hid_usage *usage);
> +
> #endif /* _VIVALDI_KEYMAP_H */
> --
> https://chromeos.dev
>
Thanks.
--
Dmitry
Powered by blists - more mailing lists