[PATCH 0/2] HID: winwing: two teardown fixes

[PATCH 0/2] HID: winwing: two teardown fixes

[René Onier]

Two teardown bugs in hid-winwing, both present in mainline. The series is
based on hid.git for-next, which already carries the related fix
a1a5ad37e50c ("HID: winwing: fix use-after-free in force feedback
teardown"); it does not depend on it.

Patch 1 fixes a use-after-free of the rumble work item. winwing_remove()
cancels the work before it stops the device, but stopping the device flushes
the force feedback effects, which calls the driver's play_effect handler one
last time and queues the work again:

  hid_hw_stop() -> input_unregister_device() -> evdev_cleanup()
    -> input_flush_device() -> input_ff_flush() -> erase_effect()
    -> ml_ff_playback(dev, id, 0) -> ml_play_effects()
    -> winwing_play_effect() -> schedule_work(&data->rumble_work)

The data the work runs on is devm-allocated, and hid_device_remove() releases
the driver's devres group as soon as .remove returns, so the requeued work
runs on freed memory. Cancelling after hid_hw_stop() closes the window: once
the input device is gone nothing can queue the work again, and the driver
data is still valid until .remove returns.

Patch 2 initialises data->lights_lock, which winwing_led_write() has been
taking since the driver was merged. The mutex only ever gets the zeroing from
devm_kzalloc(); CONFIG_DEBUG_MUTEXES and lockdep both flag it on the first
brightness write.

Patch 1 needs a device with a rumble motor to trigger; patch 2 affects every
supported device. Both were pointed out by the automated Sashiko review of
the earlier force feedback fix on linux-input, and confirmed by reading the
teardown path rather than by a crash. I have since exercised patch 1 on URSA MINOR sticks, with the URSA
MINOR series (posted separately, on top of this one) applied: unloading the
module while a 5 s rumble effect is playing. ftrace shows the chain above
taking place inside hid_hw_stop(), and the requeued work running before
winwing_remove() returns; no warning or oops (on a kernel without KASAN).

A third, related issue is deliberately left out of this series. The LED class
devices are registered with devm_led_classdev_register(), so they outlive
hid_hw_stop() by the length of the devres pass that hid_device_remove() runs
after .remove returns. A sysfs brightness write in that window reaches
hid_hw_output_report() on a stopped device; usbhid returns an error once its
output URB pointer has been cleared, but that check is not serialised against
usbhid_stop(). Fixing it inside the driver means dropping devm for the LEDs
and unwinding them by hand in the probe error paths - a fair amount of churn
for a narrow race, and the same shape exists in other HID drivers that
register LEDs with devm, so it may belong in the HID core instead. I have a
driver-side patch ready and will post it separately if you prefer that.

René Onier (2):
  HID: winwing: fix use-after-free of the rumble work
  HID: winwing: initialize the lights_lock mutex

 drivers/hid/hid-winwing.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)


base-commit: 145c2b2e9a5c0f794fb4009bcb072ab19f8ccfcd
-- 
2.55.0

[PATCH 1/2] HID: winwing: fix use-after-free of the rumble work

[René Onier]

winwing_remove() cancels the rumble work before it tears the device
down:

	if (data)
		cancel_work_sync(&data->rumble_work);

	hid_hw_close(hdev);
	hid_hw_stop(hdev);

Nothing keeps the work from being queued again after that cancel.
hid_hw_stop() unregisters the input device, and evdev_cleanup() flushes
it while it is still open: input_flush_device() -> input_ff_flush() ->
erase_effect() -> ff->playback(dev, id, 0). On a memoryless device that
is ml_ff_playback(), which marks a playing effect as aborting and calls
ml_play_effects(), handing the driver the combined - now zeroed -
effect. That is winwing_play_effect(), and it ends with

	return schedule_work(&data->rumble_work);

Unbinding, rmmod'ing or unplugging the device while an effect is playing
therefore queues the work again, after cancel_work_sync() has run.

The driver data is devm-allocated, and hid_device_remove() releases the
driver's devres group as soon as winwing_remove() returns. The pending
work then runs on freed memory: both the work_struct itself and the
report buffer winwing_haptic_rumble() writes into live in that
allocation.

Cancel the work once the device has been stopped instead. After
hid_hw_stop() has returned the input device is gone and no further
effect can be played, so a single cancel_work_sync() is enough. The
driver data is still valid at that point, since devres only runs after
winwing_remove() has returned, so a work item that is still running
while the transport goes down touches valid memory only; its
hid_hw_output_report() call simply fails once the low-level driver has
torn its endpoints down.

Fixes: 42d020b54edc ("HID: winwing: Enable rumble effects")
Signed-off-by: René Onier <f3nr1l@me.com>
---
 drivers/hid/hid-winwing.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-winwing.c b/drivers/hid/hid-winwing.c
index 19b92c2c65..e8030fdc14 100644
--- a/drivers/hid/hid-winwing.c
+++ b/drivers/hid/hid-winwing.c
@@ -391,11 +391,16 @@ static void winwing_remove(struct hid_device *hdev)
 
 	data = (struct winwing_drv_data *) hid_get_drvdata(hdev);
 
-	if (data)
-		cancel_work_sync(&data->rumble_work);
-
 	hid_hw_close(hdev);
 	hid_hw_stop(hdev);
+
+	/*
+	 * Only cancel the work once the input device is gone: stopping the
+	 * device flushes the force feedback effects, which plays them one
+	 * last time and queues the work again.
+	 */
+	if (data)
+		cancel_work_sync(&data->rumble_work);
 }
 
 static int winwing_input_configured(struct hid_device *hdev,
-- 
2.55.0

[PATCH 2/2] HID: winwing: initialize the lights_lock mutex

[René Onier]

winwing_led_write() takes data->lights_lock, but that mutex is never
initialized. winwing_probe() allocates the driver data with
devm_kzalloc() and only initializes the work item, so the mutex is left
zeroed.

Locking an uninitialized mutex is undefined behaviour; with
CONFIG_DEBUG_MUTEXES or lockdep enabled it is reported as soon as a LED
brightness is written, either from sysfs or by a LED trigger.

Initialize the mutex in winwing_probe(), next to the work item it is
allocated with.

Fixes: 266c990debad ("HID: Add WinWing Orion2 throttle support")
Signed-off-by: René Onier <f3nr1l@me.com>
---
 drivers/hid/hid-winwing.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/hid-winwing.c b/drivers/hid/hid-winwing.c
index e8030fdc14..cbfdb9c66e 100644
--- a/drivers/hid/hid-winwing.c
+++ b/drivers/hid/hid-winwing.c
@@ -374,6 +374,7 @@ static int winwing_probe(struct hid_device *hdev,
 	data->has_grip15 = id->driver_data;
 	hid_set_drvdata(hdev, data);
 
+	mutex_init(&data->lights_lock);
 	INIT_WORK(&data->rumble_work, winwing_haptic_rumble_cb);
 
 	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
-- 
2.55.0

[PATCH 0/4] HID: winwing: add WinWing URSA MINOR sticks

[René Onier]

This series adds support for the WinWing URSA MINOR joysticks (a pair of
single-hand sticks) to hid-winwing, on top of the existing Orion 2 throttle
support.

The URSA MINOR sticks differ from the Orion 2 in three ways the series
addresses:

  - they drive a single backlight LED through the stick base (a second HID
    controller on the same endpoint, addressed by a fixed device id), not
    through the Orion 2 lighting controller;
  - they carry a single rumble motor in the grip, rather than the two
    motors of the Orion 2 grips;
  - they enumerate under their own product ids.

Patches 1 and 2 are preparatory refactors with no functional change (verified
byte-for-byte): patch 1 factors the vendor report builder out of the LED and
rumble paths and names the report fields, patch 2 makes the LED set and the
lighting controller model-dependent. Patch 3 adds the device ids and the
backlight. Patch 4 drives the single grip motor while leaving the Orion 2
two-motor path untouched.

This supersedes my earlier "HID: winwing: add support for URSA MINOR combat
joysticks", sent from my gmail address, which Jiri asked me to resend with
full paths:
https://lore.kernel.org/linux-input/20260404172641.195619-1-rene.onier@gmail.com/
The device ids and the extended button mapping it enabled are in patch 3,
now together with the backlight and the rumble motor.

The series is based on hid.git for-next and applies on top of the two-patch
series "HID: winwing: two teardown fixes":
https://lore.kernel.org/linux-input/cover.1790795726.git.f3nr1l@me.com/
The only interaction is one line of context in winwing_probe(). for-next
already carries a1a5ad37e50c ("HID: winwing: fix use-after-free in force
feedback teardown"), which the new ids need since they take the
force-feedback path.

The Fighter and Space URSA MINOR variants are electrically identical and share
these product ids; only a stick-tilt accessory differs. The Civil variant, with
fewer buttons, is likely compatible but its ids have not been verified on
hardware, so it is left out.

Tested on URSA MINOR hardware (a Space left and a Fighter right): backlight,
rumble and buttons, and unloading the module while a rumble effect plays.
I could not test the Orion 2 path myself; the two-motor rumble is unchanged
and its reports are byte-identical to before (verified), but a Tested-by on
Orion 2 would be welcome.

The vendor command names used here (SET_LEDX and the report layout) were
recovered from the vendor software's own debug logs and the command table in
its WWTHID.dll, so the report fields can be named rather than left as magic
numbers.

René Onier (4):
  HID: winwing: factor out vendor SET_LEDX report builder
  HID: winwing: make the LED set and lighting controller model-dependent
  HID: winwing: add URSA MINOR sticks
  HID: winwing: drive the URSA MINOR rumble motor

 drivers/hid/hid-winwing.c | 267 ++++++++++++++++++++++++++------------
 1 file changed, 185 insertions(+), 82 deletions(-)


base-commit: 145c2b2e9a5c0f794fb4009bcb072ab19f8ccfcd
prerequisite-patch-id: d1a20c8f9775ea37cf424f0c7026816fbfecd127
prerequisite-patch-id: e6427bc64a0650061572e3923ea44827dbd63809
-- 
2.55.0

[PATCH 1/4] HID: winwing: factor out vendor SET_LEDX report builder

[René Onier]

The LED and the rumble paths each open-code the same 14-byte vendor
report byte by byte, leaving the meaning of every field to the reader.
Name the fields and build the report in one place.

A report is addressed to one of the controllers making up the product by
a (device, family) pair: the Orion 2 lighting controller answers on
0x60/0xbe, the two grip rumble motors on 0x01/0xbf and 0x03/0xbf.

No functional change: the bytes put on the wire are identical.

Signed-off-by: René Onier <f3nr1l@me.com>
---
 drivers/hid/hid-winwing.c | 123 +++++++++++++++++++-------------------
 1 file changed, 60 insertions(+), 63 deletions(-)

diff --git a/drivers/hid/hid-winwing.c b/drivers/hid/hid-winwing.c
index cbfdb9c66e..a9bf81c19f 100644
--- a/drivers/hid/hid-winwing.c
+++ b/drivers/hid/hid-winwing.c
@@ -16,6 +16,30 @@
 
 #define MAX_REPORT 16
 
+/*
+ * Vendor output report (report ID 2, 14 bytes):
+ *
+ *	[0] report ID	[1] device	[2] family	[3..4] zero
+ *	[5] payload length	[6] opcode	[7..] arguments
+ *
+ * The device and family bytes select which of the controllers making up the
+ * product the report is addressed to; unknown reports are silently dropped.
+ */
+#define WINWING_REPORT_ID	0x02
+#define WINWING_REPORT_LEN	14
+
+#define WINWING_OP_SET_LEDX	0x49	/* arguments: LED index, value */
+#define WINWING_SET_LEDX_LEN	3
+
+/* Lighting controller of the Orion 2 throttle base */
+#define ORION2_LIGHT_DEVICE	0x60
+#define ORION2_LIGHT_FAMILY	0xbe
+
+/* Rumble motors of the TGRIP-15 grips, one report each */
+#define WINWING_STICK_FAMILY		0xbf
+#define WINWING_RUMBLE_LEFT_DEVICE	0x01
+#define WINWING_RUMBLE_RIGHT_DEVICE	0x03
+
 struct winwing_led {
 	struct led_classdev cdev;
 	struct hid_device *hdev;
@@ -47,36 +71,41 @@ struct winwing_drv_data {
 	struct winwing_led leds[];
 };
 
+/*
+ * Build a vendor report addressed to one controller and send it. The caller
+ * owns @buf and any serialization it needs.
+ *
+ * Mimicking requests captured by usbmon when the LEDs and the rumble motors
+ * are controlled by the vendor's app in a VM.
+ */
+static int winwing_send_set_ledx(struct hid_device *hdev, __u8 *buf,
+		__u8 device, __u8 family, __u8 index, __u8 value)
+{
+	memset(buf, 0, WINWING_REPORT_LEN);
+
+	buf[0] = WINWING_REPORT_ID;
+	buf[1] = device;
+	buf[2] = family;
+	buf[5] = WINWING_SET_LEDX_LEN;
+	buf[6] = WINWING_OP_SET_LEDX;
+	buf[7] = index;
+	buf[8] = value;
+
+	return hid_hw_output_report(hdev, buf, WINWING_REPORT_LEN);
+}
+
 static int winwing_led_write(struct led_classdev *cdev,
 		enum led_brightness br)
 {
 	struct winwing_led *led = (struct winwing_led *) cdev;
 	struct winwing_drv_data *data = hid_get_drvdata(led->hdev);
-	__u8 *buf = data->report_lights;
 	int ret;
 
 	mutex_lock(&data->lights_lock);
 
-	/*
-	 * Mimicking requests captured by usbmon when LEDs
-	 * are controlled by the vendor's app in a VM.
-	 */
-	buf[0] = 0x02;
-	buf[1] = 0x60;
-	buf[2] = 0xbe;
-	buf[3] = 0x00;
-	buf[4] = 0x00;
-	buf[5] = 0x03;
-	buf[6] = 0x49;
-	buf[7] = led->number;
-	buf[8] = br;
-	buf[9] = 0x00;
-	buf[10] = 0;
-	buf[11] = 0;
-	buf[12] = 0;
-	buf[13] = 0;
-
-	ret = hid_hw_output_report(led->hdev, buf, 14);
+	ret = winwing_send_set_ledx(led->hdev, data->report_lights,
+			ORION2_LIGHT_DEVICE, ORION2_LIGHT_FAMILY,
+			led->number, br);
 
 	mutex_unlock(&data->lights_lock);
 
@@ -242,28 +271,12 @@ static int winwing_haptic_rumble(struct winwing_drv_data *data)
 	if (m != data->rumble_left) {
 		int ret;
 
-		/*
-		 * Mimicking requests captured by usbmon when rumble
-		 * is activated by the vendor's app in a VM.
-		 */
-		buf[0] = 0x02;
-		buf[1] = 0x01;
-		buf[2] = 0xbf;
-		buf[3] = 0x00;
-		buf[4] = 0x00;
-		buf[5] = 0x03;
-		buf[6] = 0x49;
-		buf[7] = 0x00;
-		buf[8] = m;
-		buf[9] = 0x00;
-		buf[10] = 0;
-		buf[11] = 0;
-		buf[12] = 0;
-		buf[13] = 0;
-
-		ret = hid_hw_output_report(data->hdev, buf, 14);
+		ret = winwing_send_set_ledx(data->hdev, buf,
+				WINWING_RUMBLE_LEFT_DEVICE,
+				WINWING_STICK_FAMILY, 0, m);
 		if (ret < 0) {
-			hid_err(data->hdev, "error %d (%*ph)\n", ret, 14, buf);
+			hid_err(data->hdev, "error %d (%*ph)\n", ret,
+					WINWING_REPORT_LEN, buf);
 			return ret;
 		}
 		data->rumble_left = m;
@@ -273,28 +286,12 @@ static int winwing_haptic_rumble(struct winwing_drv_data *data)
 	if (m != data->rumble_right) {
 		int ret;
 
-		/*
-		 * Mimicking requests captured by usbmon when rumble
-		 * is activated by the vendor's app in a VM.
-		 */
-		buf[0] = 0x02;
-		buf[1] = 0x03;
-		buf[2] = 0xbf;
-		buf[3] = 0x00;
-		buf[4] = 0x00;
-		buf[5] = 0x03;
-		buf[6] = 0x49;
-		buf[7] = 0x00;
-		buf[8] = m;
-		buf[9] = 0x00;
-		buf[10] = 0;
-		buf[11] = 0;
-		buf[12] = 0;
-		buf[13] = 0;
-
-		ret = hid_hw_output_report(data->hdev, buf, 14);
+		ret = winwing_send_set_ledx(data->hdev, buf,
+				WINWING_RUMBLE_RIGHT_DEVICE,
+				WINWING_STICK_FAMILY, 0, m);
 		if (ret < 0) {
-			hid_err(data->hdev, "error %d (%*ph)\n", ret, 14, buf);
+			hid_err(data->hdev, "error %d (%*ph)\n", ret,
+					WINWING_REPORT_LEN, buf);
 			return ret;
 		}
 		data->rumble_right = m;
-- 
2.55.0

[PATCH 2/4] HID: winwing: make the LED set and lighting controller model-dependent

[René Onier]

The driver hardcodes the three LEDs of the Orion 2 throttle base and the
lighting controller they are addressed to. Other WinWing devices speaking
this protocol carry a different LED set on a different controller, so
move both to the per-device data and select them once at init time.

Turn has_grip15 into a quirk mask at the same time, so that a model can
be described by more than one bit.

No functional change: the Orion 2 keeps its three LEDs on device 0x60,
family 0xbe, and the same button mapping.

Signed-off-by: René Onier <f3nr1l@me.com>
---
 drivers/hid/hid-winwing.c | 52 ++++++++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/hid-winwing.c b/drivers/hid/hid-winwing.c
index a9bf81c19f..9ec0d2cf53 100644
--- a/drivers/hid/hid-winwing.c
+++ b/drivers/hid/hid-winwing.c
@@ -40,6 +40,9 @@
 #define WINWING_RUMBLE_LEFT_DEVICE	0x01
 #define WINWING_RUMBLE_RIGHT_DEVICE	0x03
 
+/* Grip with a rumble motor and more than 32 buttons */
+#define WINWING_GRIP15		BIT(0)
+
 struct winwing_led {
 	struct led_classdev cdev;
 	struct hid_device *hdev;
@@ -52,7 +55,7 @@ struct winwing_led_info {
 	const char *led_name;
 };
 
-static const struct winwing_led_info led_info[3] = {
+static const struct winwing_led_info orion2_led_info[] = {
 	{ 0, 255, "backlight" },
 	{ 1, 1, "a-a" },
 	{ 2, 1, "a-g" },
@@ -67,7 +70,13 @@ struct winwing_drv_data {
 	struct ff_rumble_effect rumble;
 	int rumble_left;
 	int rumble_right;
-	int has_grip15;
+	unsigned long quirks;
+
+	/* Lighting controller and LED set of this model */
+	unsigned int num_leds;
+	__u8 led_device;
+	__u8 led_family;
+
 	struct winwing_led leds[];
 };
 
@@ -104,7 +113,7 @@ static int winwing_led_write(struct led_classdev *cdev,
 	mutex_lock(&data->lights_lock);
 
 	ret = winwing_send_set_ledx(led->hdev, data->report_lights,
-			ORION2_LIGHT_DEVICE, ORION2_LIGHT_FAMILY,
+			data->led_device, data->led_family,
 			led->number, br);
 
 	mutex_unlock(&data->lights_lock);
@@ -115,10 +124,11 @@ static int winwing_led_write(struct led_classdev *cdev,
 static int winwing_init_led(struct hid_device *hdev,
 		struct input_dev *input)
 {
+	const struct winwing_led_info *table;
 	struct winwing_drv_data *data;
 	struct winwing_led *led;
-	int ret;
-	int i;
+	int ret = 0;
+	unsigned int i;
 
 	data = hid_get_drvdata(hdev);
 
@@ -130,8 +140,13 @@ static int winwing_init_led(struct hid_device *hdev,
 	if (!data->report_lights)
 		return -ENOMEM;
 
-	for (i = 0; i < 3; i += 1) {
-		const struct winwing_led_info *info = &led_info[i];
+	table = orion2_led_info;
+	data->num_leds = ARRAY_SIZE(orion2_led_info);
+	data->led_device = ORION2_LIGHT_DEVICE;
+	data->led_family = ORION2_LIGHT_FAMILY;
+
+	for (i = 0; i < data->num_leds; i += 1) {
+		const struct winwing_led_info *info = &table[i];
 
 		led = &data->leds[i];
 		led->hdev = hdev;
@@ -155,7 +170,7 @@ static int winwing_init_led(struct hid_device *hdev,
 	return ret;
 }
 
-static int winwing_map_button(int button, int has_grip15)
+static int winwing_map_button(int button, int map_more_buttons)
 {
 	if (button < 1)
 		return KEY_RESERVED;
@@ -179,7 +194,7 @@ static int winwing_map_button(int button, int has_grip15)
 		return (button - 65) + BTN_TRIGGER_HAPPY17;
 	}
 
-	if (!has_grip15) {
+	if (!map_more_buttons) {
 		/*
 		 * Not mapping numbers [33 .. 64] which
 		 * are not assigned to any real buttons
@@ -232,7 +247,7 @@ static int winwing_input_mapping(struct hid_device *hdev,
 	/* Button numbers start with 1 */
 	button = usage->hid & HID_USAGE;
 
-	code = winwing_map_button(button, data->has_grip15);
+	code = winwing_map_button(button, data->quirks & WINWING_GRIP15);
 
 	hid_map_usage(hi, usage, bit, max, EV_KEY, code);
 
@@ -368,7 +383,7 @@ static int winwing_probe(struct hid_device *hdev,
 		return -ENOMEM;
 
 	data->hdev = hdev;
-	data->has_grip15 = id->driver_data;
+	data->quirks = id->driver_data;
 	hid_set_drvdata(hdev, data);
 
 	mutex_init(&data->lights_lock);
@@ -414,18 +429,21 @@ static int winwing_input_configured(struct hid_device *hdev,
 	if (ret)
 		hid_err(hdev, "led init failed\n");
 
-	if (data->has_grip15)
+	if (data->quirks & WINWING_GRIP15)
 		winwing_init_ff(hdev, hidinput);
 
 	return ret;
 }
 
-/* Set driver_data to 1 for grips with rumble motor and more than 32 buttons */
 static const struct hid_device_id winwing_devices[] = {
-	{ HID_USB_DEVICE(0x4098, 0xbd65), .driver_data = 1 },  /* TGRIP-15E  */
-	{ HID_USB_DEVICE(0x4098, 0xbd64), .driver_data = 1 },  /* TGRIP-15EX */
-	{ HID_USB_DEVICE(0x4098, 0xbe68), .driver_data = 0 },  /* TGRIP-16EX */
-	{ HID_USB_DEVICE(0x4098, 0xbe62), .driver_data = 0 },  /* TGRIP-18   */
+	{ HID_USB_DEVICE(0x4098, 0xbd65),      /* TGRIP-15E  */
+	  .driver_data = WINWING_GRIP15 },
+	{ HID_USB_DEVICE(0x4098, 0xbd64),      /* TGRIP-15EX */
+	  .driver_data = WINWING_GRIP15 },
+	{ HID_USB_DEVICE(0x4098, 0xbe68),      /* TGRIP-16EX */
+	  .driver_data = 0 },
+	{ HID_USB_DEVICE(0x4098, 0xbe62),      /* TGRIP-18   */
+	  .driver_data = 0 },
 	{}
 };
 
-- 
2.55.0

[PATCH 3/4] HID: winwing: add URSA MINOR sticks

[René Onier]

The URSA MINOR is a single-handed stick sold as a left/right pair, each
hand enumerating as its own USB device. It speaks the same vendor
protocol as the Orion 2 throttle, so reuse the LED code for it.

A stick exposes two controllers on the same endpoint, both named by the
vendor software: the base, J5_BASE, which answers on device 0x20, family
0xbb, and drives a single backlight, and the grip, JGRIP_F1_L or
JGRIP_F1_R, which carries the rumble motor. The a-a and a-g indicators of
the Orion 2 belong to an F/A-18 throttle panel and have no counterpart on
a stick, so give the sticks their own LED table.

The grips carry more than 32 buttons and so take the extended button
mapping as well. Their rumble motor is driven by a report the two-motor
throttle path does not send, and is added separately.

Signed-off-by: René Onier <f3nr1l@me.com>
---
 drivers/hid/hid-winwing.c | 54 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 47 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-winwing.c b/drivers/hid/hid-winwing.c
index 9ec0d2cf53..a52e7efece 100644
--- a/drivers/hid/hid-winwing.c
+++ b/drivers/hid/hid-winwing.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
 /*
- * HID driver for WinWing Orion 2 throttle
+ * HID driver for WinWing Orion 2 throttle and URSA MINOR sticks
  *
  * Copyright (c) 2023 Ivan Gorinov
  */
@@ -35,6 +35,16 @@
 #define ORION2_LIGHT_DEVICE	0x60
 #define ORION2_LIGHT_FAMILY	0xbe
 
+/*
+ * Each URSA MINOR stick exposes two controllers on the same endpoint. The
+ * vendor software names them J5_BASE (0xbb20), the base, which carries the
+ * backlight, and JGRIP_F1_L and JGRIP_F1_R (0xbf09 and 0xbf0a), the grips,
+ * which carry the rumble motor. Each of these ids is the family byte followed
+ * by the device byte. The base answers on the same id on either hand.
+ */
+#define URSA_LIGHT_DEVICE	0x20
+#define URSA_LIGHT_FAMILY	0xbb
+
 /* Rumble motors of the TGRIP-15 grips, one report each */
 #define WINWING_STICK_FAMILY		0xbf
 #define WINWING_RUMBLE_LEFT_DEVICE	0x01
@@ -42,6 +52,8 @@
 
 /* Grip with a rumble motor and more than 32 buttons */
 #define WINWING_GRIP15		BIT(0)
+/* URSA MINOR stick, one USB device per hand */
+#define WINWING_URSA_MINOR	BIT(1)
 
 struct winwing_led {
 	struct led_classdev cdev;
@@ -61,6 +73,17 @@ static const struct winwing_led_info orion2_led_info[] = {
 	{ 2, 1, "a-g" },
 };
 
+/*
+ * The URSA MINOR sticks have a single monochrome backlight. The a-a and a-g
+ * indicators of the Orion 2 belong to an F/A-18 throttle panel and have no
+ * counterpart here.
+ */
+static const struct winwing_led_info ursa_led_info[] = {
+	{ 0, 255, "backlight" },
+};
+
+#define WINWING_MAX_LEDS	ARRAY_SIZE(orion2_led_info)
+
 struct winwing_drv_data {
 	struct hid_device *hdev;
 	struct mutex lights_lock;
@@ -140,10 +163,17 @@ static int winwing_init_led(struct hid_device *hdev,
 	if (!data->report_lights)
 		return -ENOMEM;
 
-	table = orion2_led_info;
-	data->num_leds = ARRAY_SIZE(orion2_led_info);
-	data->led_device = ORION2_LIGHT_DEVICE;
-	data->led_family = ORION2_LIGHT_FAMILY;
+	if (data->quirks & WINWING_URSA_MINOR) {
+		table = ursa_led_info;
+		data->num_leds = ARRAY_SIZE(ursa_led_info);
+		data->led_device = URSA_LIGHT_DEVICE;
+		data->led_family = URSA_LIGHT_FAMILY;
+	} else {
+		table = orion2_led_info;
+		data->num_leds = ARRAY_SIZE(orion2_led_info);
+		data->led_device = ORION2_LIGHT_DEVICE;
+		data->led_family = ORION2_LIGHT_FAMILY;
+	}
 
 	for (i = 0; i < data->num_leds; i += 1) {
 		const struct winwing_led_info *info = &table[i];
@@ -368,7 +398,7 @@ static int winwing_probe(struct hid_device *hdev,
 		const struct hid_device_id *id)
 {
 	struct winwing_drv_data *data;
-	size_t data_size = struct_size(data, leds, 3);
+	size_t data_size = struct_size(data, leds, WINWING_MAX_LEDS);
 	int ret;
 
 	ret = hid_parse(hdev);
@@ -444,6 +474,16 @@ static const struct hid_device_id winwing_devices[] = {
 	  .driver_data = 0 },
 	{ HID_USB_DEVICE(0x4098, 0xbe62),      /* TGRIP-18   */
 	  .driver_data = 0 },
+	/*
+	 * The Fighter and Space variants are electrically identical and share
+	 * these product ids; only the stick tilt accessory differs. The Civil
+	 * variant, which has fewer buttons, is likely compatible, but its
+	 * product ids have not been verified on hardware.
+	 */
+	{ HID_USB_DEVICE(0x4098, 0xbc2a),      /* URSA MINOR R */
+	  .driver_data = WINWING_GRIP15 | WINWING_URSA_MINOR },
+	{ HID_USB_DEVICE(0x4098, 0xbc29),      /* URSA MINOR L */
+	  .driver_data = WINWING_GRIP15 | WINWING_URSA_MINOR },
 	{}
 };
 
@@ -459,5 +499,5 @@ static struct hid_driver winwing_driver = {
 };
 module_hid_driver(winwing_driver);
 
-MODULE_DESCRIPTION("HID driver for WinWing Orion 2 throttle");
+MODULE_DESCRIPTION("HID driver for WinWing Orion 2 throttle and URSA MINOR sticks");
 MODULE_LICENSE("GPL");
-- 
2.55.0

[PATCH 4/4] HID: winwing: drive the URSA MINOR rumble motor

[René Onier]

An URSA MINOR stick has a single rumble motor, carried by the grip rather
than by the base and addressed by its own device number, derived from the
product id. The two-motor throttle path sends to device 0x01 and 0x03,
which a stick ignores, so give it its own path.

Both magnitudes feed the one motor and the stronger one wins, which is
what the memoryless force-feedback core expects of a device that cannot
drive them separately.

The Orion 2 keeps its two motors.

Signed-off-by: René Onier <f3nr1l@me.com>
---
 drivers/hid/hid-winwing.c | 50 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-winwing.c b/drivers/hid/hid-winwing.c
index a52e7efece..86caff9183 100644
--- a/drivers/hid/hid-winwing.c
+++ b/drivers/hid/hid-winwing.c
@@ -4,6 +4,7 @@
  * HID driver for WinWing Orion 2 throttle and URSA MINOR sticks
  *
  * Copyright (c) 2023 Ivan Gorinov
+ * Copyright (c) 2026 René Onier
  */
 
 #include <linux/device.h>
@@ -45,8 +46,10 @@
 #define URSA_LIGHT_DEVICE	0x20
 #define URSA_LIGHT_FAMILY	0xbb
 
-/* Rumble motors of the TGRIP-15 grips, one report each */
+/* Rumble motors answer on the grip, not on the base */
 #define WINWING_STICK_FAMILY		0xbf
+
+/* The TGRIP-15 grips have two motors, each taking its own report */
 #define WINWING_RUMBLE_LEFT_DEVICE	0x01
 #define WINWING_RUMBLE_RIGHT_DEVICE	0x03
 
@@ -95,6 +98,10 @@ struct winwing_drv_data {
 	int rumble_right;
 	unsigned long quirks;
 
+	/* URSA MINOR: single motor, on the grip */
+	__u8 rumble_device;
+	int rumble_sent;
+
 	/* Lighting controller and LED set of this model */
 	unsigned int num_leds;
 	__u8 led_device;
@@ -296,6 +303,40 @@ static inline int convert_magnitude(int x)
 	return ((x * 255) >> 16) + 1;
 }
 
+/* Device number of the grip, 0x09 for the left stick and 0x0a for the right */
+static inline __u8 winwing_ursa_device(__u32 product)
+{
+	return (product & 0xff) - 0x20;
+}
+
+/*
+ * An URSA MINOR stick has a single motor, driven by the same opcode as the
+ * lights but addressed to the grip instead of the base. Both magnitudes feed
+ * it and the stronger one wins.
+ */
+static int winwing_ursa_rumble(struct winwing_drv_data *data, __u8 *buf)
+{
+	int ret;
+	__u8 m;
+
+	m = convert_magnitude(max(data->rumble.strong_magnitude,
+				data->rumble.weak_magnitude));
+
+	if (m == data->rumble_sent)
+		return 0;
+
+	ret = winwing_send_set_ledx(data->hdev, buf, data->rumble_device,
+			WINWING_STICK_FAMILY, 0, m);
+	if (ret < 0) {
+		hid_err(data->hdev, "error %d (%*ph)\n", ret,
+				WINWING_REPORT_LEN, buf);
+		return ret;
+	}
+	data->rumble_sent = m;
+
+	return 0;
+}
+
 static int winwing_haptic_rumble(struct winwing_drv_data *data)
 {
 	__u8 *buf;
@@ -312,6 +353,9 @@ static int winwing_haptic_rumble(struct winwing_drv_data *data)
 	if (!buf)
 		return -EINVAL;
 
+	if (data->quirks & WINWING_URSA_MINOR)
+		return winwing_ursa_rumble(data, buf);
+
 	m = convert_magnitude(data->rumble.strong_magnitude);
 	if (m != data->rumble_left) {
 		int ret;
@@ -382,6 +426,10 @@ static int winwing_init_ff(struct hid_device *hdev, struct hid_input *hidinput)
 	data->report_rumble = devm_kzalloc(&hdev->dev, MAX_REPORT, GFP_KERNEL);
 	data->rumble_left = -1;
 	data->rumble_right = -1;
+	data->rumble_sent = -1;
+
+	if (data->quirks & WINWING_URSA_MINOR)
+		data->rumble_device = winwing_ursa_device(hdev->product);
 
 	input_set_capability(hidinput->input, EV_FF, FF_RUMBLE);
 
-- 
2.55.0

Re: [PATCH 0/2] HID: winwing: two teardown fixes

[René Onier]

A note for whoever picks this up: patch 1 touches the same lines of
winwing_remove() as Guangshuo Li's "HID: winwing: remove unpaired
hid_hw_close()", posted on 13 September:
https://lore.kernel.org/linux-input/20260913143909.1589019-1-lgs201920130244@gmail.com/

The two changes are compatible. With both applied, the end of
winwing_remove() becomes:

	hid_hw_stop(hdev);

	/* ...comment from patch 1... */
	if (data)
		cancel_work_sync(&data->rumble_work);

Whichever goes in first, I am happy to rebase mine on top of it.

René