Skip to content

feat(t500rs): add USB interrupt-transfer driver; Proton/Steam working… #175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cazzoo wants to merge 4 commits into from
Closed

feat(t500rs): add USB interrupt-transfer driver; Proton/Steam working… #175

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
be8c39d
hid: tmff2: add Thrustmaster T500RS wheel base driver
cazzoo Nov 4, 2025
dcb3704
Merge candidate/pr175 feedbacks (#9)
cazzoo Nov 12, 2025
9c8c1f0
Candidate/driver alignment on par other drivers and captures (#11)
cazzoo Nov 12, 2025
768afca
Another pass after comments review
cazzoo Nov 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Kbuild
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ hid-tmff-new-y := \
src/tmt300rs/hid-tmt300rs.o \
src/tmt248/hid-tmt248.o \
src/tmtx/hid-tmtx.o \
src/tmtsxw/hid-tmtsxw.o
src/tmtsxw/hid-tmtsxw.o \
src/tmt500rs/hid-tmt500rs-usb.o

# Pass through the T500RS version define from Makefile (original branch style)
ccflags-y += $(T500RS_VERSION_DEF)
11 changes: 11 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
KDIR ?= /lib/modules/$(shell uname -r)/build

# Auto-generated build-time version for T500RS
Copy link
Owner

@Kimplul Kimplul Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makefile is for all wheels, either make this be generic or just remove as I don't fully see the point of a miles long version string.

Out of curiosity, why do think it would be unfortunate to remove? Does this version string have any benefit that the DKMS version doesn't?

Copy link
Owner

@Kimplul Kimplul Nov 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was marked resolved without a comment as to why, and I don't see an answer to my question in the second paragraph:

Out of curiosity, why do think it would be unfortunate to remove? Does this version string have any benefit that the DKMS version doesn't?

T500RS_BASE_VERSION ?= 0.1
GIT_HASH := $(shell git rev-parse --short=7 HEAD 2>/dev/null || echo "local")
BUILD_HASH := $(shell date +%s | sha1sum | cut -c1-7)

T500RS_VERSION ?= $(T500RS_BASE_VERSION)-$(GIT_HASH)+b$(BUILD_HASH)
export T500RS_VERSION_DEF := -DT500RS_DRIVER_VERSION=\"$(T500RS_VERSION)\"


all: deps/hid-tminit
@echo "T500RS build version: $(T500RS_VERSION)"
@echo " - base: $(T500RS_BASE_VERSION), commit: $(GIT_HASH), build: $(BUILD_HASH)"
$(MAKE) -C $(KDIR) M=$(shell pwd) modules

install: deps/hid-tminit
Expand Down
111 changes: 105 additions & 6 deletions src/hid-tmff2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,22 @@
#include <linux/version.h>
#include "hid-tmff2.h"

/* Share t500rs_log_level across compilation units (level 3 = unknown-only) */
extern int t500rs_log_level;

/* Known vendor/opcode IDs managed by the driver (TX side) */
static inline int tmff2_is_known_vendor_id(unsigned char id)
{
switch (id) {
case 0x01: case 0x02: case 0x03: case 0x04: case 0x05:
case 0x40: case 0x41: case 0x42: case 0x43:
case 0x0a:
return 1;
default:
return 0;
}
}


int open_mode = 1;
module_param(open_mode, int, 0660);
Expand Down Expand Up @@ -242,8 +258,15 @@ static ssize_t gain_store(struct device *dev,
}

gain = value;
if (tmff2->set_gain) /* if we can, update gain immediately */
tmff2->set_gain(tmff2->data, (GAIN_MAX * gain) / GAIN_MAX);
if (tmff2->set_gain) {
unsigned long flags;
spin_lock_irqsave(&tmff2->lock, flags);
tmff2->pending_gain_value = GAIN_MAX;
tmff2->gain_pending = 1;
spin_unlock_irqrestore(&tmff2->lock, flags);
if (!delayed_work_pending(&tmff2->work) && tmff2->allow_scheduling)
schedule_delayed_work(&tmff2->work, 0);
}

return count;
}
Expand All @@ -258,6 +281,7 @@ static DEVICE_ATTR_RW(gain);
static void tmff2_set_gain(struct input_dev *dev, uint16_t value)
{
struct tmff2_device_entry *tmff2 = tmff2_from_input(dev);
unsigned long flags;

if (!tmff2)
return;
Expand All @@ -267,13 +291,20 @@ static void tmff2_set_gain(struct input_dev *dev, uint16_t value)
return;
}

if (tmff2->set_gain(tmff2->data, (value * gain) / GAIN_MAX))
hid_warn(tmff2->hdev, "unable to set gain\n");
/* Defer to workqueue: store pending gain and schedule */
spin_lock_irqsave(&tmff2->lock, flags);
tmff2->pending_gain_value = value;
tmff2->gain_pending = 1;
spin_unlock_irqrestore(&tmff2->lock, flags);

if (!delayed_work_pending(&tmff2->work) && tmff2->allow_scheduling)
schedule_delayed_work(&tmff2->work, 0);
}

static void tmff2_set_autocenter(struct input_dev *dev, uint16_t value)
{
struct tmff2_device_entry *tmff2 = tmff2_from_input(dev);
unsigned long flags;

if (!tmff2)
return;
Expand All @@ -283,8 +314,14 @@ static void tmff2_set_autocenter(struct input_dev *dev, uint16_t value)
return;
}

if (tmff2->set_autocenter(tmff2->data, value))
hid_warn(tmff2->hdev, "unable to set autocenter\n");
/* Defer to workqueue: store pending autocenter and schedule */
spin_lock_irqsave(&tmff2->lock, flags);
tmff2->pending_autocenter_value = value;
tmff2->autocenter_pending = 1;
spin_unlock_irqrestore(&tmff2->lock, flags);

if (!delayed_work_pending(&tmff2->work) && tmff2->allow_scheduling)
schedule_delayed_work(&tmff2->work, 0);
}

static void tmff2_work_handler(struct work_struct *w)
Expand All @@ -301,6 +338,30 @@ static void tmff2_work_handler(struct work_struct *w)
if (!tmff2)
return;

/* Apply pending control changes (gain/autocenter) in process context */
{
unsigned long f2;
uint16_t pg = 0, pac = 0;
int do_gain = 0, do_ac = 0;
spin_lock_irqsave(&tmff2->lock, f2);
if (tmff2->gain_pending) {
pg = tmff2->pending_gain_value;
tmff2->gain_pending = 0;
do_gain = 1;
}
if (tmff2->autocenter_pending) {
pac = tmff2->pending_autocenter_value;
tmff2->autocenter_pending = 0;
do_ac = 1;
}
spin_unlock_irqrestore(&tmff2->lock, f2);

if (do_gain && tmff2->set_gain)
tmff2->set_gain(tmff2->data, (pg * gain) / GAIN_MAX);
if (do_ac && tmff2->set_autocenter)
tmff2->set_autocenter(tmff2->data, pac);
}

for (effect_id = 0; effect_id < tmff2->max_effects; ++effect_id) {
unsigned long actions = 0;
struct tmff2_effect_state effect;
Expand Down Expand Up @@ -694,6 +755,11 @@ static int tmff2_probe(struct hid_device *hdev, const struct hid_device_id *id)
goto wheel_err;
break;

case TMT500RS_PC_ID:
if ((ret = t500rs_populate_api(tmff2)))
goto wheel_err;
break;

case TMT248_PC_ID:
if ((ret = t248_populate_api(tmff2)))
goto wheel_err;
Expand Down Expand Up @@ -743,6 +809,7 @@ static int tmff2_probe(struct hid_device *hdev, const struct hid_device_id *id)

#if LINUX_VERSION_CODE < KERNEL_VERSION(6,12,0)
static __u8 *tmff2_report_fixup(struct hid_device *hdev, __u8 *rdesc,

unsigned int *rsize)
#else
static const __u8 *tmff2_report_fixup(struct hid_device *hdev, __u8 *rdesc,
Expand Down Expand Up @@ -778,6 +845,7 @@ static void tmff2_remove(struct hid_device *hdev)
if (tmff2->params & PARAM_DAMPER_LEVEL)
device_remove_file(dev, &dev_attr_damper_level);


if (tmff2->params & PARAM_SPRING_LEVEL)
device_remove_file(dev, &dev_attr_spring_level);

Expand All @@ -802,6 +870,8 @@ static const struct hid_device_id tmff2_devices[] = {
{HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, TMT300RS_PS3_NORM_ID)},
{HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, TMT300RS_PS3_ADV_ID)},
{HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, TMT300RS_PS4_NORM_ID)},
/* t500rs */
{HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, TMT500RS_PC_ID)},
/* t248 PC*/
{HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, TMT248_PC_ID)},
/* tx */
Expand All @@ -812,13 +882,42 @@ static const struct hid_device_id tmff2_devices[] = {
{}
};
MODULE_DEVICE_TABLE(hid, tmff2_devices);
static int tmff2_raw_event(struct hid_device *hdev, struct hid_report *report,
__u8 *data, int size)
{
/* At level 3: ignore normal input reports (axes/buttons) and idless reports;
* only dump vendor/feature-like unknowns. */
if (t500rs_log_level >= 3 && report && data && size > 0) {
/* Skip input traffic entirely (these are the steering updates you saw). */
if (report->type == HID_INPUT_REPORT)
return 0;

/* Use the real Report ID, not data[0] (id==0 means no Report ID). */
if (report->id == 0)
return 0;

if (!tmff2_is_known_vendor_id((unsigned char)report->id)) {
char hex[3 * 64 + 4];
int i, off = 0, max = size > 64 ? 64 : size;
for (i = 0; i < max && off + 3 < sizeof(hex); i++)
off += scnprintf(hex + off, sizeof(hex) - off, "%02x ", data[i]);
if (size > max)
scnprintf(hex + off, sizeof(hex) - off, "...");
hid_info(hdev, "HID RX UNKNOWN [type=%d id=0x%02x len=%d]: %s\n",
report->type, report->id, size, hex);
}
}
return 0; /* always pass through */
}


static struct hid_driver tmff2_driver = {
.name = "tmff2",
.id_table = tmff2_devices,
.probe = tmff2_probe,
.remove = tmff2_remove,
.report_fixup = tmff2_report_fixup,
.raw_event = tmff2_raw_event,
};
module_hid_driver(tmff2_driver);

Expand Down
14 changes: 14 additions & 0 deletions src/hid-tmff2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ struct tmff2_device_entry {

spinlock_t lock;

/* Pending control changes to be applied from workqueue context */
Copy link
Owner

@Kimplul Kimplul Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these really have to applied from within the workqueue? That's somewhat surprising to me. If they do, I would prefer that they were added as FF_*_FLAG, as the value can just be read from the global value directly and this extra step in between is unnecessary.

Copy link
Author

@cazzoo cazzoo Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While using usb interrupts, the workqueue deferral IS necessary because:

  • The backend set_gain() and set_autocenter() functions call blocking USB operations (hid_hw_request(), usb_interrupt_msg())
  • These cannot be called from atomic/interrupt context
  • The FFB callbacks ( tmff2_set_gain, tmff2_set_autocenter) can be called from atomic context
  • Therefore, we must defer to a workqueue (process context)

We cannot use just flags without storing values because:

  • For gain: The value parameter from the FFB API is the application's requested gain, which is multiplied with the global gain module parameter: (ffb_value * global_gain) / GAIN_MAX. We need to store the FFB value to apply this formula in the workqueue.
  • For autocenter: The value parameter from the FFB API needs to be stored to avoid race conditions (the callback could be called again before the workqueue processes the first request).

Copy link
Owner

@Kimplul Kimplul Nov 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FFB callbacks ( tmff2_set_gain, tmff2_set_autocenter) can be called from atomic context

In what context would that happen? I tried following the kernel call path that ends up at tmff2_set_autocenter and didn't see any spinlocks and it's not an interrupt handler, so I'm a bit confused.

Regardless, you still don't use the FF_*_FLAG approach I wanted to see, and I don't think your response clarifies why.

uint16_t pending_gain_value;
uint16_t pending_autocenter_value;
int gain_pending;
int autocenter_pending;

int allow_scheduling;

/* fields relevant to each actual device (T300, T248...) */
Expand Down Expand Up @@ -92,6 +98,10 @@ struct tmff2_device_entry {
ssize_t (*alt_mode_show)(void *data, char *buf);
ssize_t (*alt_mode_store)(void *data, const char *buf, size_t count);
int (*set_autocenter)(void *data, uint16_t autocenter);
int (*set_spring_level)(void *data, u8 level);
int (*set_damper_level)(void *data, u8 level);
int (*set_friction_level)(void *data, u8 level);

__u8 *(*wheel_fixup)(struct hid_device *hdev, __u8 *rdesc, unsigned int *rsize);

/* void pointers are dangerous, I know, but in this case likely the
Expand All @@ -100,6 +110,7 @@ struct tmff2_device_entry {

/* external */
int t300rs_populate_api(struct tmff2_device_entry *tmff2);
int t500rs_populate_api(struct tmff2_device_entry *tmff2);
int t248_populate_api(struct tmff2_device_entry *tmff2);
int tx_populate_api(struct tmff2_device_entry *tmff2);
int tsxw_populate_api(struct tmff2_device_entry *tmff2);
Expand All @@ -108,6 +119,9 @@ int tsxw_populate_api(struct tmff2_device_entry *tmff2);
#define TMT300RS_PS3_ADV_ID 0xb66f
#define TMT300RS_PS4_NORM_ID 0xb66d

#define TMT500RS_INIT_ID 0xb65d
#define TMT500RS_PC_ID 0xb65e

#define TMT248_PC_ID 0xb696

#define TX_ACTIVE 0xb669
Expand Down
Loading