All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kenneth Albanowski <kenalba@google.com>
To: "open list:HID CORE LAYER" <linux-input@vger.kernel.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>,
	Jiri Kosina <jikos@kernel.org>,
	Peter Hutterer <peter.hutterer@who-t.net>,
	Jason Gerecke <jason.gerecke@wacom.com>,
	Kenneth Albanowski <kenalba@google.com>
Subject: [PATCH 2/3] [hid] Enable processing of fields larger than 32 bits.
Date: Wed, 19 May 2021 17:22:48 -0700	[thread overview]
Message-ID: <20210519143836.2.I62308b62b6ebddf086c125fbf1a5aa159bda891a@changeid> (raw)
In-Reply-To: <20210520002249.361821-1-kenalba@google.com>

Extends hid-core to process HID fields larger than 32 bits,
by storing them as multiple consecutive 32-bit values.

hid-input and hid-debug are specifically changed to match.
Currently, no other hid consumers get an interface to see
fields larger than 32 bits.

All existing code, including hid-input processing, will by
default see the least significant 32-bits of the field, so they
have unchanged behaviour (sign extension does not change: for
any field longer than 32 bits, extension was already a no-op,
so the least significant 32-bit value will be identical. The most
significant value, at the far end, will now be extended to fill
its 32-bit value.)

Logical min/max interaction with larger fields is limited,
as min/max and other item parameters can only be described with
32-bit values (sometimes signed). hid-input is expected to
ignore min/max in scenarios where a specific larger field is
involved.

hid-debug 'events' debugfs text report format is changed, but
only for HID fields larger than 32 bits.

Signed-off-by: Kenneth Albanowski <kenalba@google.com>
---

 Documentation/hid/hiddev.rst |  6 ++-
 drivers/hid/hid-core.c       | 80 ++++++++++++++++++++++++++----------
 drivers/hid/hid-debug.c      | 27 ++++++++----
 drivers/hid/hid-input.c      | 10 ++++-
 include/linux/hid-debug.h    |  4 +-
 include/linux/hid.h          |  2 +-
 6 files changed, 92 insertions(+), 37 deletions(-)

diff --git a/Documentation/hid/hiddev.rst b/Documentation/hid/hiddev.rst
index 209e6ba4e019e..f22fcf1b55b9e 100644
--- a/Documentation/hid/hiddev.rst
+++ b/Documentation/hid/hiddev.rst
@@ -72,8 +72,10 @@ The hiddev API uses a read() interface, and a set of ioctl() calls.
 
 HID devices exchange data with the host computer using data
 bundles called "reports".  Each report is divided into "fields",
-each of which can have one or more "usages".  In the hid-core,
-each one of these usages has a single signed 32 bit value.
+each of which can have one or more "usages". Each of these usages
+has a value, usually a 32 bit or smaller signed value. (The
+hid-core can process larger values, but these are not currently
+exposed through hiddev.)
 
 read():
 -------
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 19b2b0aae0c7e..e588c3a35a593 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -86,22 +86,35 @@ struct hid_report *hid_register_report(struct hid_device *device,
 }
 EXPORT_SYMBOL_GPL(hid_register_report);
 
+// How many 32-bit values are needed to store a field?
+static unsigned hid_field_size_in_values(unsigned flags, unsigned size_in_bits)
+{
+	if (!(flags & HID_MAIN_ITEM_VARIABLE)) {
+		return 1;
+	} else {
+		return DIV_ROUND_UP(size_in_bits, 32);
+	}
+}
+
 /*
  * Register a new field for this report.
  */
 
-static struct hid_field *hid_register_field(struct hid_report *report, unsigned usages)
+static struct hid_field *hid_register_field(struct hid_report *report, unsigned usages, unsigned flags, unsigned size_in_bits)
 {
 	struct hid_field *field;
+	unsigned size_in_values;
 
 	if (report->maxfield == HID_MAX_FIELDS) {
 		hid_err(report->device, "too many fields in report\n");
 		return NULL;
 	}
 
+	size_in_values = hid_field_size_in_values(flags, size_in_bits);
+
 	field = kzalloc((sizeof(struct hid_field) +
 			 usages * sizeof(struct hid_usage) +
-			 usages * sizeof(unsigned)), GFP_KERNEL);
+			 usages * size_in_values * sizeof(s32)), GFP_KERNEL);
 	if (!field)
 		return NULL;
 
@@ -300,7 +313,7 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
 	usages = max_t(unsigned, parser->local.usage_index,
 				 parser->global.report_count);
 
-	field = hid_register_field(report, usages);
+	field = hid_register_field(report, usages, flags, parser->global.report_size);
 	if (!field)
 		return 0;
 
@@ -1340,6 +1353,9 @@ static u32 s32ton(__s32 value, unsigned n)
  * While the USB HID spec allows unlimited length bit fields in "report
  * descriptors", most devices never use more than 16 bits.
  * One model of UPS is claimed to report "LINEV" as a 32-bit field.
+ * Some digitizers report stylus transducer IDs as 64-bit fields.
+ * The outer routines will extract multiple 32-bit parts if necessary
+ * to retrieve an entire field.
  * Search linux-kernel and linux-usb-devel archives for "hid-core extract".
  */
 
@@ -1495,16 +1511,19 @@ static int hid_match_usage(struct hid_device *hid, struct hid_usage *usage)
 }
 
 static void hid_process_event(struct hid_device *hid, struct hid_field *field,
-		struct hid_usage *usage, __s32 value, int interrupt)
+		struct hid_usage *usage, const __s32 * values, unsigned value_count, int interrupt)
 {
 	struct hid_driver *hdrv = hid->driver;
 	int ret;
 
+	if (unlikely(value_count == 0))
+		return;
+
 	if (!list_empty(&hid->debug_list))
-		hid_dump_input(hid, usage, value);
+		hid_dump_input(hid, usage, values, value_count);
 
 	if (hdrv && hdrv->event && hid_match_usage(hid, usage)) {
-		ret = hdrv->event(hid, field, usage, value);
+		ret = hdrv->event(hid, field, usage, values[0]);
 		if (ret != 0) {
 			if (ret < 0)
 				hid_err(hid, "%s's event failed with %d\n",
@@ -1514,9 +1533,9 @@ static void hid_process_event(struct hid_device *hid, struct hid_field *field,
 	}
 
 	if (hid->claimed & HID_CLAIMED_INPUT)
-		hidinput_hid_event(hid, field, usage, value);
+		hidinput_hid_event(hid, field, usage, values, value_count);
 	if (hid->claimed & HID_CLAIMED_HIDDEV && interrupt && hid->hiddev_hid_event)
-		hid->hiddev_hid_event(hid, field, usage, value);
+		hid->hiddev_hid_event(hid, field, usage, values[0]);
 }
 
 /*
@@ -1528,24 +1547,41 @@ static void hid_process_event(struct hid_device *hid, struct hid_field *field,
 static void hid_input_field(struct hid_device *hid, struct hid_field *field,
 			    __u8 *data, int interrupt)
 {
-	unsigned n;
+	unsigned n, v;
 	unsigned count = field->report_count;
 	unsigned offset = field->report_offset;
-	unsigned size = field->report_size;
+	unsigned size_in_bits = field->report_size;
+	unsigned size_in_values; /* storage size in 32-bit values, always >= 1 */
+	unsigned last_value_size_in_bits; /* bits in most significant/last value */
+	unsigned bit_pos;
 	__s32 min = field->logical_minimum;
 	__s32 max = field->logical_maximum;
 	__s32 *value;
+	static const __s32 zero = 0;
+	static const __s32 one = 1;
+
+	size_in_values = hid_field_size_in_values(field->flags, size_in_bits);
+	last_value_size_in_bits = (size_in_bits % 32) ?: 32;
 
-	value = kmalloc_array(count, sizeof(__s32), GFP_ATOMIC);
+	value = kmalloc_array(count * size_in_values, sizeof(__s32), GFP_ATOMIC);
 	if (!value)
 		return;
 
-	for (n = 0; n < count; n++) {
+	for (n = 0; n < count * size_in_values; n += size_in_values) {
+		v = 0;
+		bit_pos = offset + (n * size_in_bits);
+
+		// Extract least significant values for fields longer than 32 bits.
+		if (size_in_values > 1) {
+			for (; v < size_in_values - 1; v++, bit_pos += 32)
+				value[n+v] = hid_field_extract(hid, data, bit_pos, 32);
+		}
 
-		value[n] = min < 0 ?
-			snto32(hid_field_extract(hid, data, offset + n * size,
-			       size), size) :
-			hid_field_extract(hid, data, offset + n * size, size);
+		// May need to sign extend the most significant value.
+		value[n+v] = min < 0 ?
+			sign_extend32(hid_field_extract(hid, data, bit_pos,
+				last_value_size_in_bits), last_value_size_in_bits) :
+			hid_field_extract(hid, data, bit_pos, last_value_size_in_bits);
 
 		/* Ignore report if ErrorRollOver */
 		if (!(field->flags & HID_MAIN_ITEM_VARIABLE) &&
@@ -1555,10 +1591,10 @@ static void hid_input_field(struct hid_device *hid, struct hid_field *field,
 			goto exit;
 	}
 
-	for (n = 0; n < count; n++) {
+	for (n = 0; n < count * size_in_values; n += size_in_values) {
 
 		if (HID_MAIN_ITEM_VARIABLE & field->flags) {
-			hid_process_event(hid, field, &field->usage[n], value[n], interrupt);
+			hid_process_event(hid, field, &field->usage[n], value + n, size_in_values, interrupt);
 			continue;
 		}
 
@@ -1566,16 +1602,16 @@ static void hid_input_field(struct hid_device *hid, struct hid_field *field,
 			&& field->value[n] - min < field->maxusage
 			&& field->usage[field->value[n] - min].hid
 			&& search(value, field->value[n], count))
-				hid_process_event(hid, field, &field->usage[field->value[n] - min], 0, interrupt);
+				hid_process_event(hid, field, &field->usage[field->value[n] - min], &zero, 1, interrupt);
 
 		if (value[n] >= min && value[n] <= max
 			&& value[n] - min < field->maxusage
 			&& field->usage[value[n] - min].hid
 			&& search(field->value, value[n], count))
-				hid_process_event(hid, field, &field->usage[value[n] - min], 1, interrupt);
+				hid_process_event(hid, field, &field->usage[value[n] - min], &one, 1, interrupt);
 	}
 
-	memcpy(field->value, value, count * sizeof(__s32));
+	memcpy(field->value, value, count * size_in_values * sizeof(__s32));
 exit:
 	kfree(value);
 }
@@ -1662,7 +1698,7 @@ int hid_set_field(struct hid_field *field, unsigned offset, __s32 value)
 
 	size = field->report_size;
 
-	hid_dump_input(field->report->device, field->usage + offset, value);
+	hid_dump_input(field->report->device, field->usage + offset, &value, 1);
 
 	if (offset >= field->report_count) {
 		hid_err(field->report->device, "offset (%d) exceeds report_count (%d)\n",
diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index 9453147d020db..bc25b0e8b6b63 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -692,21 +692,30 @@ void hid_dump_report(struct hid_device *hid, int type, u8 *data,
 }
 EXPORT_SYMBOL_GPL(hid_dump_report);
 
-void hid_dump_input(struct hid_device *hdev, struct hid_usage *usage, __s32 value)
+void hid_dump_input(struct hid_device *hdev, struct hid_usage *usage, const __s32 *values, unsigned value_count)
 {
 	char *buf;
 	int len;
+	unsigned n;
 
-	buf = hid_resolv_usage(usage->hid, NULL);
-	if (!buf)
-		return;
-	len = strlen(buf);
-	snprintf(buf + len, HID_DEBUG_BUFSIZE - len - 1, " = %d\n", value);
+	for (n = 0; n < value_count; n++) {
 
-	hid_debug_event(hdev, buf);
+		buf = hid_resolv_usage(usage->hid, NULL);
+		if (!buf)
+			return;
 
-	kfree(buf);
-	wake_up_interruptible(&hdev->debug_wait);
+		len = strlen(buf);
+
+		if (value_count == 1)
+			snprintf(buf + len, HID_DEBUG_BUFSIZE - len - 1, " = %d\n", values[n]);
+		else
+			snprintf(buf + len, HID_DEBUG_BUFSIZE - len - 1, "[%d] = %d (%08x)\n", n, values[n], values[n]);
+
+		hid_debug_event(hdev, buf);
+
+		kfree(buf);
+		wake_up_interruptible(&hdev->debug_wait);
+	}
 }
 EXPORT_SYMBOL_GPL(hid_dump_input);
 
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index bceccd75b488e..ee9e8d31a45ba 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1273,10 +1273,18 @@ static void hidinput_handle_scroll(struct hid_usage *usage,
 	input_event(input, EV_REL, usage->code, hi_res);
 }
 
-void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct hid_usage *usage, __s32 value)
+void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct hid_usage *usage, const __s32 *values, unsigned value_count)
 {
 	struct input_dev *input;
 	unsigned *quirks = &hid->quirks;
+	__s32 value;
+
+	if (unlikely(value_count == 0))
+		return;
+
+	// The majority of this code was writen to only understand 32-bit sized
+	// values, and anything larger was truncated: we continue that tradition.
+	value = values[0];
 
 	if (!usage->type)
 		return;
diff --git a/include/linux/hid-debug.h b/include/linux/hid-debug.h
index ea7b23d13bfdf..b4fb1a73817b4 100644
--- a/include/linux/hid-debug.h
+++ b/include/linux/hid-debug.h
@@ -16,7 +16,7 @@
 #define HID_DEBUG_BUFSIZE 512
 #define HID_DEBUG_FIFOSIZE 512
 
-void hid_dump_input(struct hid_device *, struct hid_usage *, __s32);
+void hid_dump_input(struct hid_device *, struct hid_usage *, const __s32 *, unsigned);
 void hid_dump_report(struct hid_device *, int , u8 *, int);
 void hid_dump_device(struct hid_device *, struct seq_file *);
 void hid_dump_field(struct hid_field *, int, struct seq_file *);
@@ -37,7 +37,7 @@ struct hid_debug_list {
 
 #else
 
-#define hid_dump_input(a,b,c)		do { } while (0)
+#define hid_dump_input(a,b,c,d)		do { } while (0)
 #define hid_dump_report(a,b,c,d)	do { } while (0)
 #define hid_dump_device(a,b)		do { } while (0)
 #define hid_dump_field(a,b,c)		do { } while (0)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 59828a6080e83..8494b1995b10b 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -878,7 +878,7 @@ extern void hid_unregister_driver(struct hid_driver *);
 	module_driver(__hid_driver, hid_register_driver, \
 		      hid_unregister_driver)
 
-extern void hidinput_hid_event(struct hid_device *, struct hid_field *, struct hid_usage *, __s32);
+extern void hidinput_hid_event(struct hid_device *, struct hid_field *, struct hid_usage *, const __s32 *values, unsigned value_count);
 extern void hidinput_report_event(struct hid_device *hid, struct hid_report *report);
 extern int hidinput_connect(struct hid_device *hid, unsigned int force);
 extern void hidinput_disconnect(struct hid_device *);
-- 
2.31.0


  parent reply	other threads:[~2021-05-20  0:23 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-20  0:22 [PATCH 0/3] 64-bit Digitizer Serial Numbers Kenneth Albanowski
2021-05-20  0:22 ` [PATCH 1/3] [hid] Minor cleanup Kenneth Albanowski
2021-05-20  0:22 ` Kenneth Albanowski [this message]
2021-05-20  0:22 ` [PATCH 3/3] [hid] Emit digitizer serial number through power_supply Kenneth Albanowski
2021-06-09 20:52 ` [PATCH 0/3] 64-bit Digitizer Serial Numbers Kenneth Albanowski
2021-09-15 14:56 ` Jiri Kosina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210519143836.2.I62308b62b6ebddf086c125fbf1a5aa159bda891a@changeid \
    --to=kenalba@google.com \
    --cc=benjamin.tissoires@redhat.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jason.gerecke@wacom.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=peter.hutterer@who-t.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.