[PATCH] HID: logitech-hidpp: add native touchpad support for K400 Plus

[PATCH] HID: logitech-hidpp: add native touchpad support for K400 Plus

[Dmitriy Geels]

The Logitech K400 Plus (WPID 0x404d) exposes its touchpad through the
HID++ TouchpadRawXY feature, but currently operates as an emulated mouse.

Add support for the native raw touchpad reports by creating a separate
multitouch input device while leaving the keyboard and physical button
reports on the existing HID input device.

The K400 Plus reports an area range of zero. Do not expose
ABS_MT_PRESSURE when the TouchpadRawXY feature reports no contact area,
as a permanently zero pressure value causes userspace to treat contacts
as hovering rather than touching.

This enables native touchpad handling, including multitouch gestures and
two-finger scrolling.

This is an AI-assisted patch.

Signed-off-by: Dmitriy Geels <dmitriy.geels@gmail.com>
---

Tested with Logitech K400 Plus, WPID 0x404d, connected through a
Unifying receiver.

Tested:
- single-finger pointer motion
- two-finger gestures
- physical buttons
- device power cycle
- suspend/resume

 drivers/hid/hid-logitech-hidpp.c | 133 +++++++++++++++++++++++--------
 1 file changed, 100 insertions(+), 33 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 1504de32b1c8..3cb239d81888 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -64,6 +64,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
 #define HIDPP_QUIRK_CLASS_K400			BIT(2)
 #define HIDPP_QUIRK_CLASS_G920			BIT(3)
 #define HIDPP_QUIRK_CLASS_K750			BIT(4)
+#define HIDPP_QUIRK_CLASS_KBD_WTP		BIT(5)
 
 /* bits 2..20 are reserved for classes */
 /* #define HIDPP_QUIRK_CONNECT_EVENTS		BIT(21) disabled */
@@ -2932,11 +2933,13 @@ static int hidpp_ff_init(struct hidpp_device *hidpp,
 #define WTP_MANUAL_RESOLUTION				39
 
 struct wtp_data {
+	struct input_dev *input;
 	u16 x_size, y_size;
 	u8 finger_count;
 	u8 mt_feature_index;
 	u8 button_feature_index;
 	u8 maxcontacts;
+	u8 area_range;
 	bool flip_y;
 	unsigned int resolution;
 };
@@ -2953,6 +2956,8 @@ static void wtp_populate_input(struct hidpp_device *hidpp,
 {
 	struct wtp_data *wd = hidpp->private_data;
 
+	wd->input = input_dev;
+
 	__set_bit(EV_ABS, input_dev->evbit);
 	__set_bit(EV_KEY, input_dev->evbit);
 	__clear_bit(EV_REL, input_dev->evbit);
@@ -2963,8 +2968,13 @@ static void wtp_populate_input(struct hidpp_device *hidpp,
 	input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, wd->y_size, 0, 0);
 	input_abs_set_res(input_dev, ABS_MT_POSITION_Y, wd->resolution);
 
-	/* Max pressure is not given by the devices, pick one */
-	input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 50, 0, 0);
+	/*
+	 * The driver exposes contact area as pressure.  Do not advertise a
+	 * pressure axis when the device does not report contact area; userspace
+	 * may otherwise interpret a permanently-zero pressure value as hover.
+	 */
+	if (wd->area_range)
+		input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 50, 0, 0);
 
 	input_set_capability(input_dev, EV_KEY, BTN_LEFT);
 
@@ -2987,25 +2997,28 @@ static void wtp_touch_event(struct hidpp_device *hidpp,
 		/* no actual data */
 		return;
 
-	slot = input_mt_get_slot_by_key(hidpp->input, touch_report->finger_id);
+	slot = input_mt_get_slot_by_key(wd->input, touch_report->finger_id);
 
-	input_mt_slot(hidpp->input, slot);
-	input_mt_report_slot_state(hidpp->input, MT_TOOL_FINGER,
-					touch_report->contact_status);
+	input_mt_slot(wd->input, slot);
+	input_mt_report_slot_state(wd->input, MT_TOOL_FINGER,
+				   touch_report->contact_status);
 	if (touch_report->contact_status) {
-		input_event(hidpp->input, EV_ABS, ABS_MT_POSITION_X,
-				touch_report->x);
-		input_event(hidpp->input, EV_ABS, ABS_MT_POSITION_Y,
-				wd->flip_y ? wd->y_size - touch_report->y :
-					     touch_report->y);
-		input_event(hidpp->input, EV_ABS, ABS_MT_PRESSURE,
-				touch_report->area);
+		input_event(wd->input, EV_ABS, ABS_MT_POSITION_X,
+			    touch_report->x);
+		input_event(wd->input, EV_ABS, ABS_MT_POSITION_Y,
+			    wd->flip_y ? wd->y_size - touch_report->y :
+			    touch_report->y);
+
+		if (wd->area_range)
+			input_event(wd->input, EV_ABS, ABS_MT_PRESSURE,
+				    touch_report->area);
 	}
 }
 
 static void wtp_send_raw_xy_event(struct hidpp_device *hidpp,
 		struct hidpp_touchpad_raw_xy *raw)
 {
+	struct wtp_data *wd = hidpp->private_data;
 	int i;
 
 	for (i = 0; i < 2; i++)
@@ -3013,11 +3026,11 @@ static void wtp_send_raw_xy_event(struct hidpp_device *hidpp,
 
 	if (raw->end_of_frame &&
 	    !(hidpp->quirks & HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS))
-		input_event(hidpp->input, EV_KEY, BTN_LEFT, raw->button);
+		input_event(wd->input, EV_KEY, BTN_LEFT, raw->button);
 
 	if (raw->end_of_frame || raw->finger_count <= 2) {
-		input_mt_sync_frame(hidpp->input);
-		input_sync(hidpp->input);
+		input_mt_sync_frame(wd->input);
+		input_sync(wd->input);
 	}
 }
 
@@ -3067,7 +3080,7 @@ static int wtp_raw_event(struct hid_device *hdev, u8 *data, int size)
 	struct hidpp_report *report = (struct hidpp_report *)data;
 	struct hidpp_touchpad_raw_xy raw;
 
-	if (!wd || !hidpp->input)
+	if (!wd || !wd->input)
 		return 1;
 
 	switch (data[0]) {
@@ -3078,11 +3091,11 @@ static int wtp_raw_event(struct hid_device *hdev, u8 *data, int size)
 			return 1;
 		}
 		if (hidpp->quirks & HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS) {
-			input_event(hidpp->input, EV_KEY, BTN_LEFT,
-					!!(data[1] & 0x01));
-			input_event(hidpp->input, EV_KEY, BTN_RIGHT,
-					!!(data[1] & 0x02));
-			input_sync(hidpp->input);
+			input_event(wd->input, EV_KEY, BTN_LEFT,
+				    !!(data[1] & 0x01));
+			input_event(wd->input, EV_KEY, BTN_RIGHT,
+				    !!(data[1] & 0x02));
+			input_sync(wd->input);
 			return 0;
 		} else {
 			if (size < 21)
@@ -3109,6 +3122,9 @@ static int wtp_get_config(struct hidpp_device *hidpp)
 	struct hidpp_touchpad_raw_info raw_info = {0};
 	int ret;
 
+	if (wd->x_size)
+		return 0;
+
 	ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_TOUCHPAD_RAW_XY,
 		&wd->mt_feature_index);
 	if (ret)
@@ -3123,6 +3139,7 @@ static int wtp_get_config(struct hidpp_device *hidpp)
 	wd->x_size = raw_info.x_size;
 	wd->y_size = raw_info.y_size;
 	wd->maxcontacts = raw_info.maxcontacts;
+	wd->area_range = raw_info.area_range;
 	wd->flip_y = raw_info.origin == TOUCHPAD_RAW_XY_ORIGIN_LOWER_LEFT;
 	wd->resolution = raw_info.res;
 	if (!wd->resolution)
@@ -3146,22 +3163,27 @@ static int wtp_allocate(struct hid_device *hdev, const struct hid_device_id *id)
 	return 0;
 };
 
+static int wtp_enable_raw_mode(struct hidpp_device *hidpp)
+{
+	struct wtp_data *wd = hidpp->private_data;
+
+	return hidpp_touchpad_set_raw_report_state(hidpp,
+						  wd->mt_feature_index,
+						  true, true);
+}
+
 static int wtp_connect(struct hid_device *hdev)
 {
 	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
-	struct wtp_data *wd = hidpp->private_data;
 	int ret;
 
-	if (!wd->x_size) {
-		ret = wtp_get_config(hidpp);
-		if (ret) {
-			hid_err(hdev, "Can not get wtp config: %d\n", ret);
-			return ret;
-		}
+	ret = wtp_get_config(hidpp);
+	if (ret) {
+		hid_err(hdev, "cannot get WTP config: %d\n", ret);
+		return ret;
 	}
 
-	return hidpp_touchpad_set_raw_report_state(hidpp, wd->mt_feature_index,
-			true, true);
+	return wtp_enable_raw_mode(hidpp);
 }
 
 /* ------------------------------------------------------------------------- */
@@ -4274,6 +4296,9 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
 
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		return wtp_raw_event(hdev, data, size);
+	else if ((hidpp->quirks & HIDPP_QUIRK_CLASS_KBD_WTP) &&
+		 data[0] == REPORT_ID_HIDPP_LONG)
+		return wtp_raw_event(hdev, data, size);
 	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
 		return m560_raw_event(hdev, data, size);
 
@@ -4524,7 +4549,45 @@ static void hidpp_connect_event(struct work_struct *work)
 		return;
 	}
 
-	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_KBD_WTP) {
+		struct wtp_data *wd = hidpp->private_data;
+
+		ret = wtp_get_config(hidpp);
+		if (ret) {
+			hid_err(hdev, "cannot get WTP config: %d\n", ret);
+			return;
+		}
+
+		if (!wd->input) {
+			input = hidpp_allocate_input(hdev);
+			if (!input) {
+				hid_err(hdev, "cannot allocate touchpad input device\n");
+				return;
+			}
+
+			devm_name = devm_kasprintf(&hdev->dev, GFP_KERNEL,
+						   "%s Touchpad", hidpp->name);
+			if (!devm_name)
+				return;
+
+			input->name = devm_name;
+
+			wtp_populate_input(hidpp, input);
+
+			ret = input_register_device(input);
+			if (ret) {
+				hid_err(hdev,
+					"cannot register touchpad input device: %d\n",
+					ret);
+				wd->input = NULL;
+				return;
+			}
+		}
+
+		ret = wtp_enable_raw_mode(hidpp);
+		if (ret)
+			return;
+	} else if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
 		ret = wtp_connect(hdev);
 		if (ret)
 			return;
@@ -4755,7 +4818,8 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	    hidpp_application_equals(hdev, HID_GD_KEYBOARD))
 		hidpp->quirks |= HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS;
 
-	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
+	if (hidpp->quirks &
+	    (HIDPP_QUIRK_CLASS_WTP | HIDPP_QUIRK_CLASS_KBD_WTP)) {
 		ret = wtp_allocate(hdev, id);
 		if (ret)
 			return ret;
@@ -4885,6 +4949,9 @@ static const struct hid_device_id hidpp_devices[] = {
 	{ /* Keyboard logitech K400 */
 	  LDJ_DEVICE(0x4024),
 	  .driver_data = HIDPP_QUIRK_CLASS_K400 },
+	{ /* Wireless Touch Keyboard Logitech K400 Plus */
+	  LDJ_DEVICE(0x404d),
+	  .driver_data = HIDPP_QUIRK_CLASS_KBD_WTP },
 	{ /* Solar Keyboard Logitech K750 */
 	  LDJ_DEVICE(0x4002),
 	  .driver_data = HIDPP_QUIRK_CLASS_K750 },

base-commit: a8fcb3dbf9024da44f1614c42ea16001f4b860b0
-- 
2.55.0

Re: [PATCH] HID: logitech-hidpp: add native touchpad support for K400 Plus

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [High] Early assignment of wd->input creates a Time-of-Check to Time-of-Use (TOCTOU) race condition with the interrupt handler, leading to a NULL pointer dereference.
- [Medium] Repeated connection failures in the hidpp_connect_event workqueue lead to a devres memory leak due to missing input_free_device() on error paths.
- [Medium] The K400 Plus touchpad device incorrectly advertises INPUT_PROP_BUTTONPAD and emits conflicting BTN_LEFT events.

Pre-existing issues:
- [High] The return value of input_mt_init_slots is ignored, potentially registering a broken MT device.
- [High] Missing cancel_work_sync for reset_hi_res_work in the probe error path leads to a Use-After-Free.
--

--- Patch [1]: [PATCH] HID: logitech-hidpp: add native touchpad support for K400 Plus ---
commit f41a7d9a8fed9a9f7dba87c18d14bd22f84ca93d
Author: Dmitriy Geels <dmitriy.geels@gmail.com>

HID: logitech-hidpp: add native touchpad support for K400 Plus

This patch adds native raw touchpad support for the Logitech K400 Plus by
creating a separate multitouch input device. It ensures physical button
reports remain on the existing HID input device and disables ABS_MT_PRESSURE
when contact area is zero to prevent hover interpretation.

> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 1504de32b1c84..3cb239d818882 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c

[ ... ]

> @@ -2953,6 +2956,8 @@ static void wtp_populate_input(struct hidpp_device *hidpp,
>  {
>  	struct wtp_data *wd = hidpp->private_data;
>  
> +	wd->input = input_dev;
> +

[Severity: High]
Could this early assignment create a race condition? If an interrupt arrives
here, wtp_raw_event() will see wd->input as non-NULL. If
input_register_device() subsequently fails in hidpp_connect_event(), it
sets wd->input to NULL. If wtp_raw_event() uses the pointer exactly when it
is cleared, could this cause a NULL pointer dereference in input_event()?

>  	__set_bit(EV_ABS, input_dev->evbit);
>  	__set_bit(EV_KEY, input_dev->evbit);
>  	__clear_bit(EV_REL, input_dev->evbit);

[ ... ]

> @@ -2963,8 +2968,13 @@ static void wtp_populate_input(struct hidpp_device *hidpp,
>  	input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, wd->y_size, 0, 0);
>  	input_abs_set_res(input_dev, ABS_MT_POSITION_Y, wd->resolution);
>  
> -	/* Max pressure is not given by the devices, pick one */
> -	input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 50, 0, 0);
> +	/*
> +	 * The driver exposes contact area as pressure.  Do not advertise a
> +	 * pressure axis when the device does not report contact area; userspace
> +	 * may otherwise interpret a permanently-zero pressure value as hover.
> +	 */
> +	if (wd->area_range)
> +		input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 50, 0, 0);
>  
>  	input_set_capability(input_dev, EV_KEY, BTN_LEFT);

[Severity: Medium]
Further down in this function, devices without HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS
are assigned INPUT_PROP_BUTTONPAD. Since the K400 Plus uses
HIDPP_QUIRK_CLASS_KBD_WTP without that quirk, does this cause userspace to
misconfigure the device, given that the K400 Plus actually has discrete
physical buttons?

[Severity: High]
This is a pre-existing issue, but the return value of input_mt_init_slots()
called slightly below here is ignored. If memory allocation fails, dev->mt
remains NULL, which will cause the core to silently drop all subsequent
multitouch events.

[ ... ]

> @@ -3013,11 +3026,11 @@ static void wtp_send_raw_xy_event(struct hidpp_device *hidpp,
>  
>  	if (raw->end_of_frame &&
>  	    !(hidpp->quirks & HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS))
> -		input_event(hidpp->input, EV_KEY, BTN_LEFT, raw->button);
> +		input_event(wd->input, EV_KEY, BTN_LEFT, raw->button);

[Severity: Medium]
Could this emit conflicting BTN_LEFT events? For the K400 Plus, the
physical buttons are routed through the other interface, so raw->button
is typically 0. This might spam release events at the end of every frame
and conflict with the physical hardware buttons.

[ ... ]

> @@ -4524,7 +4549,45 @@ static void hidpp_connect_event(struct work_struct *work)
>  		return;
>  	}
>  
> -	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
> +	if (hidpp->quirks & HIDPP_QUIRK_CLASS_KBD_WTP) {
> +		struct wtp_data *wd = hidpp->private_data;

[ ... ]

> +			ret = input_register_device(input);
> +			if (ret) {
> +				hid_err(hdev,
> +					"cannot register touchpad input device: %d\n",
> +					ret);
> +				wd->input = NULL;
> +				return;
> +			}

[Severity: Medium]
If input_register_device() fails, could this leak the devres allocations for
both the input device and devm_name? Since hidpp_connect_event() is a
workqueue that can run multiple times on reconnection, these allocations
might permanently accumulate without a call to input_free_device().

[ ... ]

> @@ -4755,7 +4818,8 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	    hidpp_application_equals(hdev, HID_GD_KEYBOARD))
>  		hidpp->quirks |= HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS;
>  
> -	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
> +	if (hidpp->quirks &
> +	    (HIDPP_QUIRK_CLASS_WTP | HIDPP_QUIRK_CLASS_KBD_WTP)) {
>  		ret = wtp_allocate(hdev, id);
>  		if (ret)
>  			return ret;

[Severity: High]
This is a pre-existing issue, but earlier in hidpp_probe(),
hidpp->reset_hi_res_work is initialized. In the hid_hw_start_fail error path
(which executes after wtp_allocate), cancel_work_sync(&hidpp->work) is
called, but cancel_work_sync(&hidpp->reset_hi_res_work) is omitted. If the
devres group is freed, could a pending high-res scroll work item access
freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824130901.84600-1-dmitriy.geels@gmail.com?part=1