[PATCH RESEND 2] HID: Add force feedback support for Speedlink Cougar Vibration Flightstick

[PATCH RESEND 2] HID: Add force feedback support for Speedlink Cougar Vibration Flightstick

[Harald Judt]

Hi,

I have implemented force feedback/rumble support for the Speedlink
Cougar Vibration Flightstick joystick. While I am not quite sure about
the correctness of my approach regarding the strong and weak motors, it
seems to work as expected; I ran the fftest samples and also tested it
successfully with SuperTuxKart.

Here is the code, hopefully I have set up thunderbird mail correctly to
not mangle the patch...


From 48d8512fbe49ae7b940dc5869fe50aa905d3d5fb Mon Sep 17 00:00:00 2001
From: Harald Judt <h.judt@gmx.at>
Date: Sun, 18 Jan 2026 02:47:20 +0100
Subject: [PATCH] HID: Add force feedback support for Gembird based joystick

This commit adds force feedback support for a Gembird based joystick, namely
the SpeedLink Cougar Vibration Flightstick (SL-6630), which sports vibration
motors for rumble effects. Though it is not easy to determine, it seems to have
one motor in the base and the other in the stick, both are of equal
strength. The implementation tries to take this into account for realising weak
and strong rumble effects and has been tested using fftest and SuperTuxKart.

Signed-off-by: Harald Judt <h.judt@gmx.at>
---
 drivers/hid/Kconfig           |   8 ++
 drivers/hid/Makefile          |   1 +
 drivers/hid/hid-gembird-joy.c | 178 ++++++++++++++++++++++++++++++++++
 drivers/hid/hid-ids.h         |   3 +
 drivers/hid/hid-quirks.c      |   3 +
 5 files changed, 193 insertions(+)
 create mode 100644 drivers/hid/hid-gembird-joy.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 04420a713be0..b4e2c8f67728 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -406,6 +406,14 @@ config HID_GEMBIRD
 	help
 	Support for Gembird JPD-DualForce 2.
 
+config HID_GEMBIRD_JOY_FF
+	tristate "Gembird Joysticks force feedback support"
+	depends on USB_HID
+	select INPUT_FF_MEMLESS
+	help
+	Force feedback support for Gembird (Vendor ID 0x12bd) based devices:
+	  - Speed Link Cougar Vibration Flightstick (SL-6630)
+
 config HID_GFRM
 	tristate "Google Fiber TV Box remote control support"
 	help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 361a7daedeb8..593a429661ed 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_HID_EVISION)	+= hid-evision.o
 obj-$(CONFIG_HID_EZKEY)		+= hid-ezkey.o
 obj-$(CONFIG_HID_FT260)		+= hid-ft260.o
 obj-$(CONFIG_HID_GEMBIRD)	+= hid-gembird.o
+obj-$(CONFIG_HID_GEMBIRD_JOY_FF)	+= hid-gembird-joy.o
 obj-$(CONFIG_HID_GFRM)		+= hid-gfrm.o
 obj-$(CONFIG_HID_GLORIOUS)  += hid-glorious.o
 obj-$(CONFIG_HID_VIVALDI_COMMON) += hid-vivaldi-common.o
diff --git a/drivers/hid/hid-gembird-joy.c b/drivers/hid/hid-gembird-joy.c
new file mode 100644
index 000000000000..5a5afa02f840
--- /dev/null
+++ b/drivers/hid/hid-gembird-joy.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  HID driver for Gembird based Joysticks
+ *
+ *  Currently supported devices:
+ *    - 12bd:a02f Speed Link Cougar Vibration Flightstick (SL-6630)
+ *
+ *  Copyright (c) 2026 Harald Judt <h.judt@gmx.at>
+ */
+
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+
+struct gembird_joy_device {
+	struct hid_report *report;
+};
+
+static int hid_gembird_joy_play(struct input_dev *dev, void *data,
+				struct ff_effect *effect)
+{
+	struct hid_device *hid = input_get_drvdata(dev);
+	struct gembird_joy_device *joy = data;
+	int strong, weak;
+
+	strong = effect->u.rumble.strong_magnitude;
+	weak = effect->u.rumble.weak_magnitude;
+
+	hid_dbg(hid, "called with 0x%04x 0x%04x\n", strong, weak);
+
+	/* Likely there are two motors, one in the base and the other in an upper
+	 * part of the stick, yet they are not different in strength, and honestly
+	 * it is hard to determine which motor is active or correlates to value 0
+	 * or 1 of the report field. However, the third value does not have any
+	 * function, thus it will always be set to 0.
+	 *
+	 * The windows drivers want to make believe the first value in the report
+	 * field should be influenced by horizontal axis movement, and the second
+	 * by vertical axis movement, but this might just be some arbitrary gimmick
+	 * of the configuration dialog, which describes the motors as left and
+	 * right.
+	 *
+	 * Ranges are the same for both motors, weak and strong (0 to 31), so scale
+	 * down the magnitudes accordingly...
+	 */
+	strong = (strong / 0xff) * 0x1f / 0xff;
+	weak = (weak / 0xff) * 0x1f / 0xff;
+
+	/* ... and to support the notions of strong vs weak rumble effects,
+	 * increase the magnitude for the strong rumble effect if it is below the
+	 * half of the maximum value, as the strong motor has the same strength as
+	 * the weak one. Likewise, decrease the magnitude for the weak effect.
+	 */
+	if (strong < 0x10 && !weak)         /* fftest effect 4 strong rumble */
+		strong *= 2;
+	else if (!strong && weak >= 0x10)   /* fftest effect 5 weak rumble */
+		weak /= 2;
+
+	/* Use unmodified values if both magnitudes have been set. */
+	joy->report->field[0]->value[0] = strong;
+	joy->report->field[0]->value[1] = weak;
+	joy->report->field[0]->value[2] = 0;
+
+	hid_dbg(hid, "running with 0x%02x, 0x%02x\n", strong, weak);
+	hid_hw_request(hid, joy->report, HID_REQ_SET_REPORT);
+
+	return 0;
+}
+
+static int gembird_joy_init(struct hid_device *hid)
+{
+	struct gembird_joy_device *joy;
+	struct hid_report *report;
+	struct hid_input *hidinput;
+	struct list_head *report_list =
+			&hid->report_enum[HID_OUTPUT_REPORT].report_list;
+	struct list_head *report_ptr = report_list;
+	struct input_dev *dev;
+	int error;
+
+	if (list_empty(&hid->inputs)) {
+		hid_err(hid, "no inputs found\n");
+		return -ENODEV;
+	}
+	hidinput = list_first_entry(&hid->inputs, struct hid_input, list);
+	dev = hidinput->input;
+
+	if (list_empty(report_list)) {
+		hid_err(hid, "no output reports found\n");
+		return -ENODEV;
+	}
+
+	list_for_each_entry(hidinput, &hid->inputs, list) {
+		report_ptr = report_ptr->next;
+
+		if (report_ptr == report_list) {
+			hid_err(hid, "required output report is missing\n");
+			return -ENODEV;
+		}
+
+		report = list_entry(report_ptr, struct hid_report, list);
+		if (report->maxfield < 1) {
+			hid_err(hid, "no fields in the report\n");
+			return -ENODEV;
+		}
+
+		if (report->field[0]->report_count < 3) {
+			hid_err(hid, "not enough values in the field\n");
+			return -ENODEV;
+		}
+
+		joy = kzalloc(sizeof(struct gembird_joy_device), GFP_KERNEL);
+		if (!joy)
+			return -ENOMEM;
+
+		dev = hidinput->input;
+
+		set_bit(FF_RUMBLE, dev->ffbit);
+
+		error = input_ff_create_memless(dev, joy, hid_gembird_joy_play);
+		if (error) {
+			kfree(joy);
+			return error;
+		}
+
+		joy->report = report;
+		joy->report->field[0]->value[0] = 0x00;
+		joy->report->field[0]->value[1] = 0x00;
+		joy->report->field[0]->value[2] = 0x00;
+		hid_hw_request(hid, joy->report, HID_REQ_SET_REPORT);
+	}
+
+	hid_info(hid, "Force Feedback for Gembird Joystick devices by Harald Judt <h.judt@gmx.at>\n");
+
+	return 0;
+}
+
+static int gembird_joy_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+	int ret;
+
+	ret = hid_parse(hdev);
+	if (ret) {
+		hid_err(hdev, "parse failed\n");
+		return ret;
+	}
+
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
+	if (ret) {
+		hid_err(hdev, "hw start failed\n");
+		return ret;
+	}
+
+	gembird_joy_init(hdev);
+
+	return 0;
+}
+
+static const struct hid_device_id gembird_joy_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_GEMBIRD_JOY, USB_DEVICE_ID_GEMBIRD_JOY_SL_6630) },
+	{ }
+};
+MODULE_DEVICE_TABLE(hid, gembird_joy_devices);
+
+static struct hid_driver gembird_joy_driver = {
+	.name = "gembird_joy",
+	.id_table = gembird_joy_devices,
+	.probe = gembird_joy_probe,
+};
+module_hid_driver(gembird_joy_driver);
+
+MODULE_AUTHOR("Harald Judt <h.judt@gmx.at>");
+MODULE_DESCRIPTION("Force feedback support for HID Gembird joysticks");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index c4589075a5ed..1bdd9b879152 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -528,6 +528,9 @@
 #define USB_VENDOR_ID_GEMBIRD			0x11ff
 #define USB_DEVICE_ID_GEMBIRD_JPD_DUALFORCE2	0x3331
 
+#define USB_VENDOR_ID_GEMBIRD_JOY		0x12bd
+#define USB_DEVICE_ID_GEMBIRD_JOY_SL_6630	0xa02f
+
 #define USB_VENDOR_ID_GENERAL_TOUCH	0x0dfc
 #define USB_DEVICE_ID_GENERAL_TOUCH_WIN7_TWOFINGERS 0x0003
 #define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PWT_TENFINGERS 0x0100
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 6a8a7ca3d804..835070491da7 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -444,6 +444,9 @@ static const struct hid_device_id hid_have_special_driver[] = {
 #if IS_ENABLED(CONFIG_HID_GEMBIRD)
 	{ HID_USB_DEVICE(USB_VENDOR_ID_GEMBIRD, USB_DEVICE_ID_GEMBIRD_JPD_DUALFORCE2) },
 #endif
+#if IS_ENABLED(CONFIG_HID_GEMBIRD_JOY_FF)
+	{ HID_USB_DEVICE(USB_VENDOR_ID_GEMBIRD_JOY, USB_DEVICE_ID_GEMBIRD_JOY_SL_6630) },
+#endif
 #if IS_ENABLED(CONFIG_HID_GFRM)
 	{ HID_BLUETOOTH_DEVICE(0x58, 0x2000) },
 	{ HID_BLUETOOTH_DEVICE(0x471, 0x2210) },
-- 
2.52.0



-- 
`Experience is the best teacher.'

PGP Key ID: 4FFFAB21B8580ABD
Fingerprint: E073 6DD8 FF40 9CF2 0665 11D4 4FFF AB21 B858 0ABD

Re: [PATCH RESEND 2] HID: Add force feedback support for Speedlink Cougar Vibration Flightstick

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Race condition in force feedback initialization leading to a NULL pointer dereference.
- [High] Ignoring the return value of `gembird_joy_init()` causes a NULL pointer dereference when user-space interacts with the event device after an initialization failure.
- [High] Non-monotonic magnitude scaling logic severely breaks force feedback functionality.
- [Medium] Memory leak of the `gembird_joy_device` state structure upon device disconnect.
--

commit f9141496ab06cd22000d2487bbb234d7e91931b5
Author: Harald Judt <h.judt@gmx.at>

HID: Add force feedback support for Gembird based joystick

This commit adds force feedback (rumble) support for the SpeedLink Cougar
Vibration Flightstick (SL-6630) joystick, which uses a Gembird controller.

> diff --git a/drivers/hid/hid-gembird-joy.c b/drivers/hid/hid-gembird-joy.c
> new file mode 100644
> index 0000000000000..5a5afa02f8407
> --- /dev/null
> +++ b/drivers/hid/hid-gembird-joy.c

[ ... ]

> +static int hid_gembird_joy_play(struct input_dev *dev, void *data,
> +				struct ff_effect *effect)
> +{
> +	struct hid_device *hid = input_get_drvdata(dev);
> +	struct gembird_joy_device *joy = data;
> +	int strong, weak;
> +
> +	strong = effect->u.rumble.strong_magnitude;
> +	weak = effect->u.rumble.weak_magnitude;

[ ... ]

> +	strong = (strong / 0xff) * 0x1f / 0xff;
> +	weak = (weak / 0xff) * 0x1f / 0xff;
> +
> +	/* ... and to support the notions of strong vs weak rumble effects,
> +	 * increase the magnitude for the strong rumble effect if it is below the
> +	 * half of the maximum value, as the strong motor has the same strength as
> +	 * the weak one. Likewise, decrease the magnitude for the weak effect.
> +	 */
> +	if (strong < 0x10 && !weak)         /* fftest effect 4 strong rumble */
> +		strong *= 2;
> +	else if (!strong && weak >= 0x10)   /* fftest effect 5 weak rumble */
> +		weak /= 2;

Will this cause issues with non-monotonic force feedback behavior and
unintended truncation?

Since the math uses integer division, any input value below 255 for strong
or weak will evaluate to zero during the first division by 0xff.

Additionally, could the conditional scaling cause abrupt changes in rumble
strength? For instance, if a game requests a strong magnitude that scales
down to 15, it gets doubled to 30 because of the strong < 0x10 check.
However, a slightly stronger requested magnitude that scales to 16 will
remain 16, meaning an increase in requested strength results in a drop in
physical rumble.

[ ... ]

> +static int gembird_joy_init(struct hid_device *hid)
> +{

[ ... ]

> +		joy = kzalloc(sizeof(struct gembird_joy_device), GFP_KERNEL);
> +		if (!joy)
> +			return -ENOMEM;

Does this memory leak when the device is disconnected or the driver is
unbound? The allocation doesn't use devm_kzalloc, and there isn't a
remove callback to free it. The input_ff_create_memless teardown
handles its own structures but not the user-provided private data.

> +
> +		dev = hidinput->input;
> +
> +		set_bit(FF_RUMBLE, dev->ffbit);
> +
> +		error = input_ff_create_memless(dev, joy, hid_gembird_joy_play);
> +		if (error) {
> +			kfree(joy);
> +			return error;
> +		}
> +
> +		joy->report = report;

Is there a window here where an ioctl could trigger a NULL pointer
dereference?

The device was already started earlier in gembird_joy_probe via
hid_hw_start, so userspace can issue an EVIOCSFF ioctl as soon as
input_ff_create_memless succeeds. If the ioctl triggers
hid_gembird_joy_play before joy->report is assigned on the next line,
the callback will try to access joy->report->field and dereference NULL.

[ ... ]

> +static int gembird_joy_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
> +	int ret;
> +
> +	ret = hid_parse(hdev);
> +	if (ret) {
> +		hid_err(hdev, "parse failed\n");
> +		return ret;
> +	}
> +
> +	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
> +	if (ret) {
> +		hid_err(hdev, "hw start failed\n");
> +		return ret;
> +	}
> +
> +	gembird_joy_init(hdev);
> +
> +	return 0;
> +}

What happens if gembird_joy_init fails?

If input_ff_create_memless returns an error, gembird_joy_init
propagates it, but the return value is ignored here and probe succeeds.

Because set_bit(FF_RUMBLE, dev->ffbit) was already called, the device
remains registered and advertises rumble support. If userspace then issues
an EVIOCSFF ioctl, the kernel will allow it, which could lead to a panic
when it tries to dereference the missing force feedback structures.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/b3bfa73b-0547-45f9-b6ae-f28815e6c632@gmx.at?part=1

Re: [PATCH RESEND 2] HID: Add force feedback support for Speedlink Cougar Vibration Flightstick

[Dmitry Torokhov]

On Fri, May 08, 2026 at 10:51:30PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [High] Race condition in force feedback initialization leading to a NULL pointer dereference.
> - [High] Ignoring the return value of `gembird_joy_init()` causes a NULL pointer dereference when user-space interacts with the event device after an initialization failure.
> - [High] Non-monotonic magnitude scaling logic severely breaks force feedback functionality.
> - [Medium] Memory leak of the `gembird_joy_device` state structure upon device disconnect.
> --
> 
> commit f9141496ab06cd22000d2487bbb234d7e91931b5
> Author: Harald Judt <h.judt@gmx.at>
> 
> HID: Add force feedback support for Gembird based joystick
> 
> This commit adds force feedback (rumble) support for the SpeedLink Cougar
> Vibration Flightstick (SL-6630) joystick, which uses a Gembird controller.
> 
> > diff --git a/drivers/hid/hid-gembird-joy.c b/drivers/hid/hid-gembird-joy.c
> > new file mode 100644
> > index 0000000000000..5a5afa02f8407
> > --- /dev/null
> > +++ b/drivers/hid/hid-gembird-joy.c
> 
> [ ... ]
> 
> > +static int hid_gembird_joy_play(struct input_dev *dev, void *data,
> > +				struct ff_effect *effect)
> > +{
> > +	struct hid_device *hid = input_get_drvdata(dev);
> > +	struct gembird_joy_device *joy = data;
> > +	int strong, weak;
> > +
> > +	strong = effect->u.rumble.strong_magnitude;
> > +	weak = effect->u.rumble.weak_magnitude;
> 
> [ ... ]
> 
> > +	strong = (strong / 0xff) * 0x1f / 0xff;
> > +	weak = (weak / 0xff) * 0x1f / 0xff;
> > +
> > +	/* ... and to support the notions of strong vs weak rumble effects,
> > +	 * increase the magnitude for the strong rumble effect if it is below the
> > +	 * half of the maximum value, as the strong motor has the same strength as
> > +	 * the weak one. Likewise, decrease the magnitude for the weak effect.
> > +	 */
> > +	if (strong < 0x10 && !weak)         /* fftest effect 4 strong rumble */
> > +		strong *= 2;
> > +	else if (!strong && weak >= 0x10)   /* fftest effect 5 weak rumble */
> > +		weak /= 2;
> 
> Will this cause issues with non-monotonic force feedback behavior and
> unintended truncation?
> 
> Since the math uses integer division, any input value below 255 for strong
> or weak will evaluate to zero during the first division by 0xff.
> 
> Additionally, could the conditional scaling cause abrupt changes in rumble
> strength? For instance, if a game requests a strong magnitude that scales
> down to 15, it gets doubled to 30 because of the strong < 0x10 check.
> However, a slightly stronger requested magnitude that scales to 16 will
> remain 16, meaning an increase in requested strength results in a drop in
> physical rumble.
> 
> [ ... ]
> 
> > +static int gembird_joy_init(struct hid_device *hid)
> > +{
> 
> [ ... ]
> 
> > +		joy = kzalloc(sizeof(struct gembird_joy_device), GFP_KERNEL);
> > +		if (!joy)
> > +			return -ENOMEM;
> 
> Does this memory leak when the device is disconnected or the driver is
> unbound? The allocation doesn't use devm_kzalloc, and there isn't a
> remove callback to free it. The input_ff_create_memless teardown
> handles its own structures but not the user-provided private data.

Sashiko is wrong here.

> 
> > +
> > +		dev = hidinput->input;
> > +
> > +		set_bit(FF_RUMBLE, dev->ffbit);
> > +
> > +		error = input_ff_create_memless(dev, joy, hid_gembird_joy_play);
> > +		if (error) {
> > +			kfree(joy);
> > +			return error;
> > +		}
> > +
> > +		joy->report = report;
> 
> Is there a window here where an ioctl could trigger a NULL pointer
> dereference?
> 
> The device was already started earlier in gembird_joy_probe via
> hid_hw_start, so userspace can issue an EVIOCSFF ioctl as soon as
> input_ff_create_memless succeeds. If the ioctl triggers
> hid_gembird_joy_play before joy->report is assigned on the next line,
> the callback will try to access joy->report->field and dereference NULL.

Sashiko is wrong here as well. The device is not accessible from
userspace until it is registered.

> 
> [ ... ]
> 
> > +static int gembird_joy_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > +{
> > +	int ret;
> > +
> > +	ret = hid_parse(hdev);
> > +	if (ret) {
> > +		hid_err(hdev, "parse failed\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
> > +	if (ret) {
> > +		hid_err(hdev, "hw start failed\n");
> > +		return ret;
> > +	}
> > +
> > +	gembird_joy_init(hdev);
> > +
> > +	return 0;
> > +}
> 
> What happens if gembird_joy_init fails?
> 
> If input_ff_create_memless returns an error, gembird_joy_init
> propagates it, but the return value is ignored here and probe succeeds.
> 
> Because set_bit(FF_RUMBLE, dev->ffbit) was already called, the device
> remains registered and advertises rumble support. If userspace then issues
> an EVIOCSFF ioctl, the kernel will allow it, which could lead to a panic
> when it tries to dereference the missing force feedback structures.
> 

Need to have error handling.

Thanks.

-- 
Dmitry

Re: [PATCH RESEND 2] HID: Add force feedback support for Speedlink Cougar Vibration Flightstick

[Harald Judt]

Hi,

Am 09.05.26 um 01:14 schrieb Dmitry Torokhov:

[ ... ]

>>> +	strong = (strong / 0xff) * 0x1f / 0xff;
>>> +	weak = (weak / 0xff) * 0x1f / 0xff;
>>> +
>>> +	/* ... and to support the notions of strong vs weak rumble effects,
>>> +	 * increase the magnitude for the strong rumble effect if it is below the
>>> +	 * half of the maximum value, as the strong motor has the same strength as
>>> +	 * the weak one. Likewise, decrease the magnitude for the weak effect.
>>> +	 */
>>> +	if (strong < 0x10 && !weak)         /* fftest effect 4 strong rumble */
>>> +		strong *= 2;
>>> +	else if (!strong && weak >= 0x10)   /* fftest effect 5 weak rumble */
>>> +		weak /= 2;
>>
>> Will this cause issues with non-monotonic force feedback behavior and
>> unintended truncation?

I thought about this too. I have not experienced any negative behavior with
this solution, the stick seems to work fine with it. But honestly, there
are not many tests available. fftest and SuperTuxKart have been the only
applications I found supporting force feedback, it worked fine there,
but the stick is also a bit limited in capabilities, it does not really
support "force feedback", only some degrees of rumble.

Since I wrote this patch some time passed and I have gained a bit more
experience with force feedback in general and implementing support for it.

>> Since the math uses integer division, any input value below 255 for strong
>> or weak will evaluate to zero during the first division by 0xff.

I am aware of this but IIRC this does not have any practical impact for
this stick. It seems to correspond to the values I got from the SET_REPORTs
in Windows and the vibrating behaviour.

>> Additionally, could the conditional scaling cause abrupt changes in rumble
>> strength? For instance, if a game requests a strong magnitude that scales
>> down to 15, it gets doubled to 30 because of the strong < 0x10 check.
>> However, a slightly stronger requested magnitude that scales to 16 will
>> remain 16, meaning an increase in requested strength results in a drop in
>> physical rumble.

True. I have tested this only with fftest and observed the behaviour in
SuperTuxKart, where it worked fine, as it uses both strong and weak
attributes. Existing implementations for other devices in the kernel
source I found sometimes use a "left" motor and a "right" motor,
other times a "strong" motor and a "weak" motor. But the games do a
variation of strong vs weak. So I finally decided to somehow get it
to work with the game I could test it with. I will do more checks
with other parameters/gain and try to improve this or think about
another, better solution. I might probably need to write some
ff tests myself for this, but why not...
>> [ ... ]
>>
>>> +static int gembird_joy_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>> +{
>>> +	int ret;
>>> +
>>> +	ret = hid_parse(hdev);
>>> +	if (ret) {
>>> +		hid_err(hdev, "parse failed\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
>>> +	if (ret) {
>>> +		hid_err(hdev, "hw start failed\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	gembird_joy_init(hdev);
>>> +
>>> +	return 0;
>>> +}
>>
>> What happens if gembird_joy_init fails?
>>
>> If input_ff_create_memless returns an error, gembird_joy_init
>> propagates it, but the return value is ignored here and probe succeeds.
>>
>> Because set_bit(FF_RUMBLE, dev->ffbit) was already called, the device
>> remains registered and advertises rumble support. If userspace then issues
>> an EVIOCSFF ioctl, the kernel will allow it, which could lead to a panic
>> when it tries to dereference the missing force feedback structures.
> 
> Need to have error handling.

I have looked at some other implementations, and if I have not missed
anything, then they do it similarly (hid-gaff.c, hid-sjoy.c, hid-emsff.c,
hid-megaworld.c). I will take a closer look again and try to fix it.

> Thanks.
Thank you for your suggestions and spending your time on this review.
I will try to improve it and send a new patch when ready.

Regards,
Harald

-- 
`Experience is the best teacher.'

PGP Key ID: 4FFFAB21B8580ABD
Fingerprint: E073 6DD8 FF40 9CF2 0665 11D4 4FFF AB21 B858 0ABD

Re: [PATCH RESEND 2] HID: Add force feedback support for Speedlink Cougar Vibration Flightstick

[Jiri Kosina]

On Fri, 8 May 2026, Harald Judt wrote:

> Hi,
> 
> I have implemented force feedback/rumble support for the Speedlink
> Cougar Vibration Flightstick joystick. While I am not quite sure about
> the correctness of my approach regarding the strong and weak motors, it
> seems to work as expected; I ran the fftest samples and also tested it
> successfully with SuperTuxKart.
> 
> Here is the code, hopefully I have set up thunderbird mail correctly to
> not mangle the patch...
> 
> 
> From 48d8512fbe49ae7b940dc5869fe50aa905d3d5fb Mon Sep 17 00:00:00 2001
> From: Harald Judt <h.judt@gmx.at>
> Date: Sun, 18 Jan 2026 02:47:20 +0100
> Subject: [PATCH] HID: Add force feedback support for Gembird based joystick
> 
> This commit adds force feedback support for a Gembird based joystick, namely
> the SpeedLink Cougar Vibration Flightstick (SL-6630), which sports vibration
> motors for rumble effects. Though it is not easy to determine, it seems to have
> one motor in the base and the other in the stick, both are of equal
> strength. The implementation tries to take this into account for realising weak
> and strong rumble effects and has been tested using fftest and SuperTuxKart.
> 
> Signed-off-by: Harald Judt <h.judt@gmx.at>
> ---
>  drivers/hid/Kconfig           |   8 ++
>  drivers/hid/Makefile          |   1 +
>  drivers/hid/hid-gembird-joy.c | 178 ++++++++++++++++++++++++++++++++++
>  drivers/hid/hid-ids.h         |   3 +
>  drivers/hid/hid-quirks.c      |   3 +
>  5 files changed, 193 insertions(+)
>  create mode 100644 drivers/hid/hid-gembird-joy.c
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 04420a713be0..b4e2c8f67728 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -406,6 +406,14 @@ config HID_GEMBIRD
>  	help
>  	Support for Gembird JPD-DualForce 2.
>  
> +config HID_GEMBIRD_JOY_FF
> +	tristate "Gembird Joysticks force feedback support"
> +	depends on USB_HID
> +	select INPUT_FF_MEMLESS
> +	help
> +	Force feedback support for Gembird (Vendor ID 0x12bd) based devices:
> +	  - Speed Link Cougar Vibration Flightstick (SL-6630)
> +
>  config HID_GFRM
>  	tristate "Google Fiber TV Box remote control support"
>  	help
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 361a7daedeb8..593a429661ed 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_HID_EVISION)	+= hid-evision.o
>  obj-$(CONFIG_HID_EZKEY)		+= hid-ezkey.o
>  obj-$(CONFIG_HID_FT260)		+= hid-ft260.o
>  obj-$(CONFIG_HID_GEMBIRD)	+= hid-gembird.o
> +obj-$(CONFIG_HID_GEMBIRD_JOY_FF)	+= hid-gembird-joy.o

Would it be possible to link this support to hid-gembird if enabled?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH RESEND 2] HID: Add force feedback support for Speedlink Cougar Vibration Flightstick

[Harald Judt]

Hi,

Thanks for reviewing this.

Am 12.05.26 um 18:11 schrieb Jiri Kosina:
> On Fri, 8 May 2026, Harald Judt wrote:
[...]

>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> index 04420a713be0..b4e2c8f67728 100644
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -406,6 +406,14 @@ config HID_GEMBIRD
>>  	help
>>  	Support for Gembird JPD-DualForce 2.
>>  
>> +config HID_GEMBIRD_JOY_FF
>> +	tristate "Gembird Joysticks force feedback support"
>> +	depends on USB_HID
>> +	select INPUT_FF_MEMLESS
>> +	help
>> +	Force feedback support for Gembird (Vendor ID 0x12bd) based devices:
>> +	  - Speed Link Cougar Vibration Flightstick (SL-6630)
>> +
>>  config HID_GFRM
>>  	tristate "Google Fiber TV Box remote control support"
>>  	help
>> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
>> index 361a7daedeb8..593a429661ed 100644
>> --- a/drivers/hid/Makefile
>> +++ b/drivers/hid/Makefile
>> @@ -54,6 +54,7 @@ obj-$(CONFIG_HID_EVISION)	+= hid-evision.o
>>  obj-$(CONFIG_HID_EZKEY)		+= hid-ezkey.o
>>  obj-$(CONFIG_HID_FT260)		+= hid-ft260.o
>>  obj-$(CONFIG_HID_GEMBIRD)	+= hid-gembird.o
>> +obj-$(CONFIG_HID_GEMBIRD_JOY_FF)	+= hid-gembird-joy.o
> 
> Would it be possible to link this support to hid-gembird if enabled?

The problem I encountered is that gembird != gembird, that is,
the USB Vendor IDs differ. These are defined:

#define USB_VENDOR_ID_GEMBIRD			0x11ff
#define USB_DEVICE_ID_GEMBIRD_JPD_DUALFORCE2	0x3331

#define USB_VENDOR_ID_GEMBIRD_JOY		0x12bd
#define USB_DEVICE_ID_GEMBIRD_JOY_SL_6630	0xa02f

What would be the best way to solve this? Can my implementation
reside in separate files or should it be integrated into the
existing hid-gembird implementation? I am pretty new to
writing kernel modules, and doing it separately simply seemed
more suited for an easier start...

Regards,
Harald

-- 
`Experience is the best teacher.'

PGP Key ID: 4FFFAB21B8580ABD
Fingerprint: E073 6DD8 FF40 9CF2 0665 11D4 4FFF AB21 B858 0ABD