[PATCH] HID: magicmouse: add haptic click configuration for Magic Trackpad 2

[PATCH] HID: magicmouse: add haptic click configuration for Magic Trackpad 2

[Christian Fressl]

Apple Magic Trackpad 2 devices support persistent haptic feedback
configuration through feature reports 0x22 and 0x23. Add an opt-in
module parameter to select either the verified silent-low profile or to
disable haptic feedback.

The default remains unchanged. The report payload format is
reverse-engineered, so keep the existing payload bytes fixed and vary
only the known 24-bit feedback value.

The USB-C Trackpad exposes multiple HID interfaces. Use the one-shot
actuator output report 0x53 only to identify the interface that accepts
the persistent configuration reports; do not use it for the persistent
setting itself.

Tested on Apple Magic Trackpad USB-C 05ac:0324 with Ubuntu
6.17.0-23-generic. Compile-tested against HID for-next.

Protocol information was derived from public reverse-engineering notes,
then independently tested with local hardware.

Link: https://github.com/mwyborski/Linux-Magic-Trackpad-2-Driver/issues/28#issuecomment-451625504
Signed-off-by: Christian Fressl <christian@fressl.at>
---
 drivers/hid/hid-magicmouse.c | 101 ++++++++++++++++++++++++++++++++++-
 1 file changed, 100 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index e70bd3dc07ab..31002cb1f108 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -51,6 +51,10 @@ static bool report_undeciphered;
 module_param(report_undeciphered, bool, 0644);
 MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state field using a MSC_RAW event");
 
+static unsigned int haptic_click;
+module_param(haptic_click, uint, 0644);
+MODULE_PARM_DESC(haptic_click, "Haptic click feedback: 0=unchanged, 1=silent-low, 2=off");
+
 #define TRACKPAD2_2021_BT_VERSION 0x110
 #define TRACKPAD_2024_BT_VERSION 0x314
 
@@ -62,6 +66,17 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie
 #define DOUBLE_REPORT_ID   0xf7
 #define USB_BATTERY_TIMEOUT_SEC 60
 
+#define TRACKPAD2_HAPTIC_CLICK_REPORT_ID     0x22
+#define TRACKPAD2_HAPTIC_RELEASE_REPORT_ID   0x23
+#define TRACKPAD2_HAPTIC_ACTUATOR_REPORT_ID  0x53
+#define TRACKPAD2_HAPTIC_REPORT_LEN          14
+#define TRACKPAD2_HAPTIC_CLICK_UNCHANGED     0
+#define TRACKPAD2_HAPTIC_CLICK_SILENT_LOW    1
+#define TRACKPAD2_HAPTIC_CLICK_OFF           2
+#define TRACKPAD2_HAPTIC_SILENT_CLICK        0x000015
+#define TRACKPAD2_HAPTIC_SILENT_RELEASE      0x000010
+#define TRACKPAD2_HAPTIC_OFF                 0x000000
+
 /* These definitions are not precise, but they're close enough.  (Bits
  * 0x03 seem to indicate the aspect ratio of the touch, bits 0x70 seem
  * to be some kind of bit mask -- 0x20 may be a near-field reading,
@@ -812,6 +827,81 @@ static bool is_usb_magictrackpad2(__u32 vendor, __u32 product)
 	       product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC;
 }
 
+static bool magicmouse_is_haptic_interface(struct hid_device *hdev)
+{
+	struct hid_report_enum *report_enum;
+
+	report_enum = &hdev->report_enum[HID_OUTPUT_REPORT];
+
+	/*
+	 * The persistent haptic configuration reports are accepted as feature
+	 * reports, but are not advertised in the feature report descriptor.
+	 * Report 0x53 is the one-shot actuator output report and identifies
+	 * the HID interface that accepts the persistent reports.
+	 */
+	return report_enum->report_id_hash[TRACKPAD2_HAPTIC_ACTUATOR_REPORT_ID];
+}
+
+static int magicmouse_send_haptic_report(struct hid_device *hdev, u8 report_id,
+					 u32 feedback)
+{
+	static const u8 report_template[TRACKPAD2_HAPTIC_REPORT_LEN] = {
+		0x00, 0x01, 0x00, 0x78, 0x02, 0x00, 0x24,
+		0x30, 0x06, 0x01, 0x00, 0x18, 0x48, 0x13,
+	};
+	u8 *buf;
+	int ret;
+
+	buf = kmemdup(report_template, sizeof(report_template), GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	buf[0] = report_id;
+	buf[2] = feedback & 0xff;
+	buf[5] = (feedback >> 8) & 0xff;
+	buf[10] = (feedback >> 16) & 0xff;
+
+	ret = hid_hw_raw_request(hdev, buf[0], buf, TRACKPAD2_HAPTIC_REPORT_LEN,
+				 HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+	kfree(buf);
+
+	return ret;
+}
+
+static int magicmouse_apply_haptic_click(struct hid_device *hdev)
+{
+	u32 click_feedback;
+	u32 release_feedback;
+	int ret;
+
+	if (haptic_click == TRACKPAD2_HAPTIC_CLICK_UNCHANGED)
+		return 0;
+
+	switch (haptic_click) {
+	case TRACKPAD2_HAPTIC_CLICK_SILENT_LOW:
+		click_feedback = TRACKPAD2_HAPTIC_SILENT_CLICK;
+		release_feedback = TRACKPAD2_HAPTIC_SILENT_RELEASE;
+		break;
+	case TRACKPAD2_HAPTIC_CLICK_OFF:
+		click_feedback = TRACKPAD2_HAPTIC_OFF;
+		release_feedback = TRACKPAD2_HAPTIC_OFF;
+		break;
+	default:
+		hid_warn(hdev, "invalid haptic_click value %u\n", haptic_click);
+		return -EINVAL;
+	}
+
+	ret = magicmouse_send_haptic_report(hdev,
+					    TRACKPAD2_HAPTIC_CLICK_REPORT_ID,
+					    click_feedback);
+	if (ret < 0)
+		return ret;
+
+	return magicmouse_send_haptic_report(hdev,
+					     TRACKPAD2_HAPTIC_RELEASE_REPORT_ID,
+					     release_feedback);
+}
+
 static int magicmouse_fetch_battery(struct hid_device *hdev)
 {
 #ifdef CONFIG_HID_BATTERY_STRENGTH
@@ -894,8 +984,17 @@ static int magicmouse_probe(struct hid_device *hdev,
 
 	if (is_usb_magicmouse2(id->vendor, id->product) ||
 	    (is_usb_magictrackpad2(id->vendor, id->product) &&
-	     hdev->type != HID_TYPE_USBMOUSE))
+	     hdev->type != HID_TYPE_USBMOUSE)) {
+		if (is_usb_magictrackpad2(id->vendor, id->product) &&
+		    magicmouse_is_haptic_interface(hdev)) {
+			ret = magicmouse_apply_haptic_click(hdev);
+			if (ret < 0)
+				hid_warn(hdev,
+					 "unable to apply haptic click setting (%d)\n",
+					 ret);
+		}
 		return 0;
+	}
 
 	if (!msc->input) {
 		hid_err(hdev, "magicmouse input not registered\n");
-- 
2.43.0

Re: [PATCH] HID: magicmouse: add haptic click configuration for Magic Trackpad 2

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The module parameter `haptic_click` is read multiple times without being cached, resulting in a Time-of-Check to Time-of-Use (TOCTOU) race condition.
--

commit 160ed71822c4a0697b90debaa037251a7e50642c
Author: Christian Fressl <christian@fressl.at>

HID: magicmouse: add haptic click configuration for Magic Trackpad 2

This commit adds support for configuring the haptic feedback on Apple
Magic Trackpad 2 devices. It introduces the haptic_click module parameter
to choose between unchanged, silent-low, or off states.

Link: https://github.com/mwyborski/Linux-Magic-Trackpad-2-Driver/issues/28#issuecomment-451625504

> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> index e70bd3dc07ab7..31002cb1f1089 100644
> --- a/drivers/hid/hid-magicmouse.c
> +++ b/drivers/hid/hid-magicmouse.c

[ ... ]

> @@ -812,6 +827,81 @@ static bool is_usb_magictrackpad2(__u32 vendor, __u32 product)

[ ... ]

> +static int magicmouse_apply_haptic_click(struct hid_device *hdev)
> +{
> +	u32 click_feedback;
> +	u32 release_feedback;
> +	int ret;
> +
> +	if (haptic_click == TRACKPAD2_HAPTIC_CLICK_UNCHANGED)
> +		return 0;
> +
> +	switch (haptic_click) {

Since haptic_click is a module parameter with 0644 permissions, can it
change concurrently between the if statement and the switch statement?

If it changes from TRACKPAD2_HAPTIC_CLICK_SILENT_LOW (1) to
TRACKPAD2_HAPTIC_CLICK_UNCHANGED (0) right after the early return check,
this regression could cause the switch statement to fall through to the
default case. This would result in an incorrect warning and return -EINVAL.

Would it be safer to read the parameter into a local variable once using
READ_ONCE() to avoid this race condition?

> +	case TRACKPAD2_HAPTIC_CLICK_SILENT_LOW:
> +		click_feedback = TRACKPAD2_HAPTIC_SILENT_CLICK;
> +		release_feedback = TRACKPAD2_HAPTIC_SILENT_RELEASE;
> +		break;
> +	case TRACKPAD2_HAPTIC_CLICK_OFF:
> +		click_feedback = TRACKPAD2_HAPTIC_OFF;
> +		release_feedback = TRACKPAD2_HAPTIC_OFF;
> +		break;
> +	default:
> +		hid_warn(hdev, "invalid haptic_click value %u\n", haptic_click);
> +		return -EINVAL;
> +	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511203115.104087-1-christian@fressl.at?part=1

[PATCH v2] HID: magicmouse: add haptic click configuration for Magic Trackpad 2

[Christian Fressl]

Apple Magic Trackpad 2 devices support persistent haptic feedback
configuration through feature reports 0x22 and 0x23. Add an opt-in
module parameter to select either the verified silent-low profile or to
disable haptic feedback.

The default remains unchanged. The report payload format is
reverse-engineered, so keep the existing payload bytes fixed and vary
only the known 24-bit feedback value.

The USB-C Trackpad exposes multiple HID interfaces. Use the one-shot
actuator output report 0x53 only to identify the interface that accepts
the persistent configuration reports; do not use it for the persistent
setting itself.

Tested on Apple Magic Trackpad USB-C 05ac:0324 with Ubuntu
6.17.0-23-generic. Compile-tested against HID for-next.

Protocol information was derived from public reverse-engineering notes,
then independently tested with local hardware.

Link: https://github.com/mwyborski/Linux-Magic-Trackpad-2-Driver/issues/28#issuecomment-451625504
Signed-off-by: Christian Fressl <christian@fressl.at>
---
Changes in v2:
- Cache haptic_click with READ_ONCE() before applying the setting.

 drivers/hid/hid-magicmouse.c | 102 ++++++++++++++++++++++++++++++++++-
 1 file changed, 101 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index e70bd3dc07ab..e941c95f0703 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -51,6 +51,10 @@ static bool report_undeciphered;
 module_param(report_undeciphered, bool, 0644);
 MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state field using a MSC_RAW event");
 
+static unsigned int haptic_click;
+module_param(haptic_click, uint, 0644);
+MODULE_PARM_DESC(haptic_click, "Haptic click feedback: 0=unchanged, 1=silent-low, 2=off");
+
 #define TRACKPAD2_2021_BT_VERSION 0x110
 #define TRACKPAD_2024_BT_VERSION 0x314
 
@@ -62,6 +66,17 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie
 #define DOUBLE_REPORT_ID   0xf7
 #define USB_BATTERY_TIMEOUT_SEC 60
 
+#define TRACKPAD2_HAPTIC_CLICK_REPORT_ID     0x22
+#define TRACKPAD2_HAPTIC_RELEASE_REPORT_ID   0x23
+#define TRACKPAD2_HAPTIC_ACTUATOR_REPORT_ID  0x53
+#define TRACKPAD2_HAPTIC_REPORT_LEN          14
+#define TRACKPAD2_HAPTIC_CLICK_UNCHANGED     0
+#define TRACKPAD2_HAPTIC_CLICK_SILENT_LOW    1
+#define TRACKPAD2_HAPTIC_CLICK_OFF           2
+#define TRACKPAD2_HAPTIC_SILENT_CLICK        0x000015
+#define TRACKPAD2_HAPTIC_SILENT_RELEASE      0x000010
+#define TRACKPAD2_HAPTIC_OFF                 0x000000
+
 /* These definitions are not precise, but they're close enough.  (Bits
  * 0x03 seem to indicate the aspect ratio of the touch, bits 0x70 seem
  * to be some kind of bit mask -- 0x20 may be a near-field reading,
@@ -812,6 +827,82 @@ static bool is_usb_magictrackpad2(__u32 vendor, __u32 product)
 	       product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC;
 }
 
+static bool magicmouse_is_haptic_interface(struct hid_device *hdev)
+{
+	struct hid_report_enum *report_enum;
+
+	report_enum = &hdev->report_enum[HID_OUTPUT_REPORT];
+
+	/*
+	 * The persistent haptic configuration reports are accepted as feature
+	 * reports, but are not advertised in the feature report descriptor.
+	 * Report 0x53 is the one-shot actuator output report and identifies
+	 * the HID interface that accepts the persistent reports.
+	 */
+	return report_enum->report_id_hash[TRACKPAD2_HAPTIC_ACTUATOR_REPORT_ID];
+}
+
+static int magicmouse_send_haptic_report(struct hid_device *hdev, u8 report_id,
+					 u32 feedback)
+{
+	static const u8 report_template[TRACKPAD2_HAPTIC_REPORT_LEN] = {
+		0x00, 0x01, 0x00, 0x78, 0x02, 0x00, 0x24,
+		0x30, 0x06, 0x01, 0x00, 0x18, 0x48, 0x13,
+	};
+	u8 *buf;
+	int ret;
+
+	buf = kmemdup(report_template, sizeof(report_template), GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	buf[0] = report_id;
+	buf[2] = feedback & 0xff;
+	buf[5] = (feedback >> 8) & 0xff;
+	buf[10] = (feedback >> 16) & 0xff;
+
+	ret = hid_hw_raw_request(hdev, buf[0], buf, TRACKPAD2_HAPTIC_REPORT_LEN,
+				 HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+	kfree(buf);
+
+	return ret;
+}
+
+static int magicmouse_apply_haptic_click(struct hid_device *hdev)
+{
+	unsigned int click = READ_ONCE(haptic_click);
+	u32 click_feedback;
+	u32 release_feedback;
+	int ret;
+
+	if (click == TRACKPAD2_HAPTIC_CLICK_UNCHANGED)
+		return 0;
+
+	switch (click) {
+	case TRACKPAD2_HAPTIC_CLICK_SILENT_LOW:
+		click_feedback = TRACKPAD2_HAPTIC_SILENT_CLICK;
+		release_feedback = TRACKPAD2_HAPTIC_SILENT_RELEASE;
+		break;
+	case TRACKPAD2_HAPTIC_CLICK_OFF:
+		click_feedback = TRACKPAD2_HAPTIC_OFF;
+		release_feedback = TRACKPAD2_HAPTIC_OFF;
+		break;
+	default:
+		hid_warn(hdev, "invalid haptic_click value %u\n", click);
+		return -EINVAL;
+	}
+
+	ret = magicmouse_send_haptic_report(hdev,
+					    TRACKPAD2_HAPTIC_CLICK_REPORT_ID,
+					    click_feedback);
+	if (ret < 0)
+		return ret;
+
+	return magicmouse_send_haptic_report(hdev,
+					     TRACKPAD2_HAPTIC_RELEASE_REPORT_ID,
+					     release_feedback);
+}
+
 static int magicmouse_fetch_battery(struct hid_device *hdev)
 {
 #ifdef CONFIG_HID_BATTERY_STRENGTH
@@ -894,8 +985,17 @@ static int magicmouse_probe(struct hid_device *hdev,
 
 	if (is_usb_magicmouse2(id->vendor, id->product) ||
 	    (is_usb_magictrackpad2(id->vendor, id->product) &&
-	     hdev->type != HID_TYPE_USBMOUSE))
+	     hdev->type != HID_TYPE_USBMOUSE)) {
+		if (is_usb_magictrackpad2(id->vendor, id->product) &&
+		    magicmouse_is_haptic_interface(hdev)) {
+			ret = magicmouse_apply_haptic_click(hdev);
+			if (ret < 0)
+				hid_warn(hdev,
+					 "unable to apply haptic click setting (%d)\n",
+					 ret);
+		}
 		return 0;
+	}
 
 	if (!msc->input) {
 		hid_err(hdev, "magicmouse input not registered\n");
-- 
2.43.0