[PATCH v12 01/17] HID: asus: size feature reports from the HID report descriptor

[Denis Benato <denis.benato@linux.dev>]

The ASUS vendor protocol uses the same feature report IDs across very
different devices: on N-KEY keyboards and ROG Ally report 0x5A/0x5E are
64-byte reports, while on XG Mobile stations report 0x5E is a 300-byte
report. The driver currently hardcodes transfer lengths (16 bytes for
the handshake, FEATURE_KBD_REPORT_SIZE for reads, ROG_XGM_REPORT_SIZE
for the XG Mobile LED, ROG_ALLY_REPORT_SIZE for the MCU version read),
which mismatches what the hardware expects and would break if a new
device used yet another size for the same report ID.

Derive the transfer length of every feature report from the HID report
descriptor of the device at hand:

- add asus_report_len(), which resolves a report ID to its struct
  hid_report and returns hid_report_len();
- asus_kbd_set_report() now zero-pads short payloads to the exact
  report size and rejects payloads that do not fit the report;
- add asus_kbd_get_report() and use it from asus_kbd_init() and
  asus_kbd_get_functions() so reads are sized per report as well;
- asus_xgm_led_set() sends a minimal payload and checks the transfer
  result against the descriptor length instead of the hardcoded
  ROG_XGM_REPORT_SIZE, which is removed along with the fixed-size
  buffers in asus_kbd_set_fn_lock() and asus_kbd_set_brightness();
- mcu_request_version() sizes the 0x0d response from the descriptor
  and names the keyboard feature report FEATURE_KBD_REPORT_ID.
  ROG_ALLY_REPORT_SIZE has no remaining user and is removed.

Captured USB traffic of the same commands issued by Windows shows they
are always written at the full report length, which this change now
matches.

Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/hid/hid-asus.c | 181 ++++++++++++++++++++++++++++++-----------
 1 file changed, 135 insertions(+), 46 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index f6aa1be09ad5..327bb4ab1a0d 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -22,6 +22,7 @@
 
 #include <linux/acpi.h>
 #include <linux/cleanup.h>
+#include <linux/container_of.h>
 #include <linux/dmi.h>
 #include <linux/hid.h>
 #include <linux/module.h>
@@ -30,6 +31,7 @@
 #include <linux/usb.h> /* For to_usb_interface for T100 touchpad intf check */
 #include <linux/power_supply.h>
 #include <linux/leds.h>
+#include <linux/types.h>
 
 #include "hid-ids.h"
 
@@ -51,9 +53,6 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define FEATURE_KBD_LED_REPORT_ID1 0x5d
 #define FEATURE_KBD_LED_REPORT_ID2 0x5e
 
-#define ROG_XGM_REPORT_SIZE 300
-
-#define ROG_ALLY_REPORT_SIZE 64
 #define ROG_ALLY_X_MIN_MCU 313
 #define ROG_ALLY_MIN_MCU 319
 
@@ -582,20 +581,104 @@ static int asus_raw_event(struct hid_device *hdev,
 	return 0;
 }
 
+/**
+ * asus_find_report() - Find a report of the given type by its ID
+ * @hdev: HID device
+ * @type: report type (HID_INPUT_REPORT, HID_OUTPUT_REPORT or
+ * HID_FEATURE_REPORT)
+ * @report_id: report ID to look for
+ *
+ * Return: the report with the given ID, or NULL if the device does not
+ * declare such a report.
+ */
+static struct hid_report *asus_find_report(struct hid_device *hdev,
+					   unsigned int type, u8 report_id)
+{
+	return hdev->report_enum[type].report_id_hash[report_id];
+}
+
+/**
+ * asus_report_len() - Length of a feature report for the device
+ * @hdev: HID device
+ * @report_id: report ID of the feature report
+ *
+ * The same ASUS report ID can have different sizes depending on the device:
+ * report 0x5E is a 64-byte report on laptops but a 300-byte report on XG
+ * Mobile stations. Always size transfers from the report descriptor of the
+ * device at hand instead of hardcoding lengths.
+ *
+ * Return: the length of the feature report in bytes, or 0 if the device
+ * does not declare the report.
+ */
+static size_t asus_report_len(struct hid_device *hdev, u8 report_id)
+{
+	struct hid_report *report;
+
+	report = asus_find_report(hdev, HID_FEATURE_REPORT, report_id);
+
+	return report ? hid_report_len(report) : 0;
+}
+
 static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t buf_size)
 {
-	u8 *dmabuf __free(kfree) = kmemdup(buf, buf_size, GFP_KERNEL);
+	size_t hw_len = asus_report_len(hdev, buf[0]);
+
+	/* The payload (including the report ID) must fit the device report. */
+	if (!hw_len || buf_size > hw_len)
+		return -EINVAL;
+
+	u8 *dmabuf __free(kfree) = kzalloc(hw_len, GFP_KERNEL);
 	if (!dmabuf)
 		return -ENOMEM;
 
+	memcpy(dmabuf, buf, buf_size);
+
 	/*
-	 * The report ID should be set from the incoming buffer due to LED and key
-	 * interfaces having different pages
+	 * The report ID should be set from the incoming buffer due to LED and
+	 * key interfaces having different pages. Short payloads are zero-padded
+	 * to the exact report size the hardware expects for this report ID.
 	 */
-	return hid_hw_raw_request(hdev, buf[0], dmabuf, buf_size, HID_FEATURE_REPORT,
+	return hid_hw_raw_request(hdev, buf[0], dmabuf, hw_len, HID_FEATURE_REPORT,
 				  HID_REQ_SET_REPORT);
 }
 
+/**
+ * asus_kbd_get_report() - Read a report sized from the descriptor
+ * @hdev: HID device
+ * @report_id: report ID to read
+ * @buf: buffer to store the report in
+ * @buf_size: size of @buf
+ *
+ * The transfer is performed at the exact report size the device declares
+ * for @report_id. The device may answer with a short read, and only the
+ * bytes that fit in @buf are copied into it.
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+static int asus_kbd_get_report(struct hid_device *hdev, u8 report_id,
+			       u8 *buf, size_t buf_size)
+{
+	size_t hw_len = asus_report_len(hdev, report_id);
+	int ret;
+
+	if (!hw_len)
+		return -EINVAL;
+
+	u8 *dmabuf __free(kfree) = kzalloc(hw_len, GFP_KERNEL);
+	if (!dmabuf)
+		return -ENOMEM;
+
+	ret = hid_hw_raw_request(hdev, report_id, dmabuf, hw_len,
+				 HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+	if (ret < 0)
+		return ret;
+
+	/* The device may return less than requested (short read). */
+	memcpy(buf, dmabuf, min(buf_size, hw_len));
+
+	return 0;
+}
+
 static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
 {
 	/*
@@ -604,8 +687,13 @@ static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
 	 */
 	const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
 		     0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
+	size_t hw_len = asus_report_len(hdev, report_id);
+	size_t cmp_len;
 	int ret;
 
+	if (!hw_len)
+		return -EINVAL;
+
 	ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
 	if (ret < 0) {
 		hid_err(hdev, "Asus handshake %02x failed to send: %d\n",
@@ -613,19 +701,20 @@ static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
 		return ret;
 	}
 
-	u8 *readbuf __free(kfree) = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
+	u8 *readbuf __free(kfree) = kzalloc(hw_len, GFP_KERNEL);
 	if (!readbuf)
 		return -ENOMEM;
 
-	ret = hid_hw_raw_request(hdev, report_id, readbuf,
-				 FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
-				 HID_REQ_GET_REPORT);
+	ret = asus_kbd_get_report(hdev, report_id, readbuf, hw_len);
 	if (ret < 0) {
 		hid_warn(hdev, "Asus handshake %02x failed to receive ack: %d\n",
 			 report_id, ret);
-	} else if (memcmp(readbuf, buf, sizeof(buf)) != 0) {
-		hid_warn(hdev, "Asus handshake %02x returned invalid response: %*ph\n",
-			 report_id, FEATURE_KBD_REPORT_SIZE, readbuf);
+	} else {
+		cmp_len = min_t(size_t, sizeof(buf), hw_len);
+		if (memcmp(readbuf, buf, cmp_len) != 0) {
+			hid_warn(hdev, "Asus handshake %02x returned invalid response: %*ph\n",
+				 report_id, (int)cmp_len, readbuf);
+		}
 	}
 
 	/*
@@ -640,32 +729,35 @@ static int asus_kbd_get_functions(struct hid_device *hdev,
 				  u8 report_id)
 {
 	const u8 buf[] = { report_id, 0x05, 0x20, 0x31, 0x00, 0x08 };
-	u8 *readbuf;
+	size_t hw_len = asus_report_len(hdev, report_id);
 	int ret;
 
+	/*
+	 * Function flags are at byte 6. A shorter report does not
+	 * contain that byte.
+	 */
+	if (hw_len < 7)
+		return -EINVAL;
+
 	ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
 	if (ret < 0) {
 		hid_err(hdev, "Asus failed to send configuration command: %d\n", ret);
 		return ret;
 	}
 
-	readbuf = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
+	u8 *readbuf __free(kfree) = kzalloc(hw_len, GFP_KERNEL);
 	if (!readbuf)
 		return -ENOMEM;
 
-	ret = hid_hw_raw_request(hdev, report_id, readbuf,
-				 FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
-				 HID_REQ_GET_REPORT);
+	ret = asus_kbd_get_report(hdev, report_id, readbuf, hw_len);
 	if (ret < 0) {
 		hid_err(hdev, "Asus failed to request functions: %d\n", ret);
-		kfree(readbuf);
 		return ret;
 	}
 
 	*kbd_func = readbuf[6];
 
-	kfree(readbuf);
-	return ret;
+	return 0;
 }
 
 static int asus_kbd_disable_oobe(struct hid_device *hdev)
@@ -690,7 +782,7 @@ static int asus_kbd_disable_oobe(struct hid_device *hdev)
 
 static void asus_kbd_set_fn_lock(struct hid_device *hdev, bool enabled)
 {
-	const u8 buf[FEATURE_KBD_REPORT_SIZE] = { FEATURE_KBD_REPORT_ID, 0xd0, 0x4e, !!enabled };
+	const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xd0, 0x4e, !!enabled };
 	int ret;
 
 	ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
@@ -700,7 +792,7 @@ static void asus_kbd_set_fn_lock(struct hid_device *hdev, bool enabled)
 
 static void asus_kbd_set_brightness(struct hid_device *hdev, u8 brightness)
 {
-	const u8 buf[FEATURE_KBD_REPORT_SIZE] = {
+	const u8 buf[] = {
 		FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, brightness
 	};
 	int ret;
@@ -900,10 +992,15 @@ static int mcu_parse_version_string(const u8 *response, size_t response_size)
 
 static int mcu_request_version(struct hid_device *hdev)
 {
-	u8 *response __free(kfree) = kzalloc(ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
-	const u8 request[] = { 0x5a, 0x05, 0x03, 0x31, 0x00, 0x20 };
+	size_t hw_len = asus_report_len(hdev, FEATURE_REPORT_ID);
+	const u8 request[] = { FEATURE_KBD_REPORT_ID, 0x05, 0x03, 0x31, 0x00, 0x20 };
 	int ret;
 
+	if (!hw_len)
+		return -EINVAL;
+
+	u8 *response __free(kfree) = kzalloc(hw_len, GFP_KERNEL);
+
 	if (!response)
 		return -ENOMEM;
 
@@ -911,17 +1008,16 @@ static int mcu_request_version(struct hid_device *hdev)
 	if (ret < 0)
 		return ret;
 
-	ret = hid_hw_raw_request(hdev, FEATURE_REPORT_ID, response,
-				ROG_ALLY_REPORT_SIZE, HID_FEATURE_REPORT,
-				HID_REQ_GET_REPORT);
+	ret = hid_hw_raw_request(hdev, FEATURE_REPORT_ID, response, hw_len,
+				 HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
 	if (ret < 0)
 		return ret;
 
-	ret = mcu_parse_version_string(response, ROG_ALLY_REPORT_SIZE);
+	ret = mcu_parse_version_string(response, hw_len);
 	if (ret < 0) {
 		pr_err("Failed to parse MCU version: %d\n", ret);
 		print_hex_dump(KERN_ERR, "MCU: ", DUMP_PREFIX_NONE,
-			      16, 1, response, ROG_ALLY_REPORT_SIZE, false);
+			       16, 1, response, hw_len, false);
 	}
 
 	return ret;
@@ -958,17 +1054,12 @@ static void validate_mcu_fw_version(struct hid_device *hdev, int idProduct)
 
 static bool asus_has_report_id(struct hid_device *hdev, u16 report_id)
 {
-	struct hid_report *report;
-	int t;
+	if (report_id >= HID_MAX_IDS)
+		return false;
 
-	for (t = HID_INPUT_REPORT; t <= HID_FEATURE_REPORT; t++) {
-		list_for_each_entry(report, &hdev->report_enum[t].report_list, list) {
-			if (report->id == report_id)
-				return true;
-		}
-	}
-
-	return false;
+	return asus_find_report(hdev, HID_INPUT_REPORT, report_id) ||
+	       asus_find_report(hdev, HID_OUTPUT_REPORT, report_id) ||
+	       asus_find_report(hdev, HID_FEATURE_REPORT, report_id);
 }
 
 static int asus_kbd_register_leds(struct hid_device *hdev)
@@ -1171,17 +1262,15 @@ static int asus_battery_probe(struct hid_device *hdev)
 
 static int asus_xgm_led_set(struct led_classdev *led_cdev, enum led_brightness value)
 {
-	const u8 buf[ROG_XGM_REPORT_SIZE] = {
-		FEATURE_KBD_LED_REPORT_ID2, 0xC5, (value) ? 0x50 : 0x00
-	};
+	const u8 buf[] = { FEATURE_KBD_LED_REPORT_ID2, 0xC5, (value) ? 0x50 : 0x00 };
 	struct asus_xgm_led *xgm = container_of(led_cdev, struct asus_xgm_led, cdev);
 	int ret;
 
-	ret = asus_kbd_set_report(xgm->hdev, buf, ROG_XGM_REPORT_SIZE);
+	ret = asus_kbd_set_report(xgm->hdev, buf, sizeof(buf));
 	if (ret < 0) {
 		hid_err(xgm->hdev, "Unable to set XG mobile led state: %d\n", ret);
 		return ret;
-	} else if (ret != ROG_XGM_REPORT_SIZE) {
+	} else if (ret != (int)asus_report_len(xgm->hdev, FEATURE_KBD_LED_REPORT_ID2)) {
 		hid_err(xgm->hdev, "Unexpected partial transfer to XG mobile: %d\n", ret);
 		return -EIO;
 	}
-- 
2.47.3