diff mbox series

[v4,2/4] HID: Extract vivaldi hid feature mapping for use in hid-hammer

Message ID 20220216195901.1326924-3-swboyd@chromium.org (mailing list archive)
State Under Review
Headers show
Series Input/HID: Consolidate ChromeOS Vivaldi keyboard logic | expand

Commit Message

Stephen Boyd Feb. 16, 2022, 7:58 p.m. UTC
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@kernel.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Tested-by: "Sean O'Brien" <seobrien@chromium.org>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Zhengqiao Xia <xiazhengqiao@huaqin.corp-partner.google.com>
Acked-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Stephen Boyd <swboyd@chromium.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

Comments

Dmitry Torokhov Feb. 28, 2022, 8:01 a.m. UTC | #1
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@kernel.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Tested-by: "Sean O'Brien" <seobrien@chromium.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Zhengqiao Xia <xiazhengqiao@huaqin.corp-partner.google.com>
> Acked-by: Jiri Kosina <jkosina@suse.cz>
> Signed-off-by: Stephen Boyd <swboyd@chromium.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.
diff mbox series

Patch

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
 	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;
+
 #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 */