Merge candidate/pr175 feedbacks (#9)

Author: cazzoo

Kbuild

@@ -7,5 +7,5 @@ hid-tmff-new-y := \
 		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)
+# Pass through the global TMFF2 version define from Makefile
+ccflags-y += $(TMFF2_VERSION_DEF)

Makefile

@@ -1,17 +1,17 @@
 KDIR ?= /lib/modules/$(shell uname -r)/build
 
-# Auto-generated build-time version for T500RS
-T500RS_BASE_VERSION ?= 0.1
+# Auto-generated global build-time version for TMFF2
+TMFF2_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)\"
+TMFF2_VERSION ?= $(TMFF2_BASE_VERSION)-$(GIT_HASH)+b$(BUILD_HASH)
+export TMFF2_VERSION_DEF := -DTMFF2_DRIVER_VERSION=\"$(TMFF2_VERSION)\"
 
 
 all: deps/hid-tminit
-	@echo "T500RS build version: $(T500RS_VERSION)"
-	@echo " - base: $(T500RS_BASE_VERSION), commit: $(GIT_HASH), build: $(BUILD_HASH)"
+	@echo "TMFF2 build version: $(TMFF2_VERSION)"
+	@echo " - base: $(TMFF2_BASE_VERSION), commit: $(GIT_HASH), build: $(BUILD_HASH)"
 	$(MAKE) -C $(KDIR) M=$(shell pwd) modules
 
 install: deps/hid-tminit

docs/FFBEFFECTS.md

@@ -1025,4 +1025,4 @@ binary    hex
     ff00.0021 = 0
     ff00.0021 = 0
     ff00.0021 = 0
-```
+```

docs/FFB_T500RS.md

@@ -0,0 +1,85 @@
+## T500RS USB FFB Protocol (observed)
+
+This page documents the packet formats and ordering used by the T500RS when driven via the USB interrupt endpoint (0x01 OUT). It mirrors the style of docs/FFBEFFECTS.md for other wheels.
+
+Key rules
+- Little‑endian for 16‑bit fields
+- DMA‑safe buffer length 32 bytes; typical messages are 2, 4, 9 or 15 bytes
+- EffectID semantics (CRITICAL):
+  - All Report 0x01 main‑effect uploads MUST use EffectID = 0x00 on T500RS
+  - Report 0x41 START/STOP also uses EffectID = 0x00, except the init‑time STOP for autocenter which targets a fixed ID (15)
+  - Using per‑effect IDs with 0x01 breaks playback (constant force fails completely)
+
+Report glossary
+- 0x01 main upload (15 bytes)
+  - Layout (unknown bytes kept for completeness):
+    - b0 id = 0x01
+    - b1 effect_id = 0x00 (MUST)
+    - b2 type: 0x00 constant; 0x20..0x24 periodic/ramp
+    - b3 0x40
+    - b4 0xff (or duration L for ramp path)
+    - b5 0xff (or duration H for ramp path)
+    - b6 0x00
+    - b7 0xff
+    - b8 0xff
+    - b9 0x0e (parameter subtype)
+    - b10 0x00
+    - b11 0x1c (envelope subtype)
+    - b12 0x00
+    - b13 0x00
+    - b14 0x00
+- 0x02 envelope (9 bytes)
+  - b0 0x02, b1 0x1c, b2 0x00
+  - b3..4 attack_length (le16)
+  - b5 attack_level (0..255)
+  - b6..7 fade_length (le16)
+  - b8 fade_level (0..255)
+- 0x03 constant level (4 bytes)
+  - b0 0x03, b1 0x0e, b2 0x00
+  - b3 level s8 (−127..127)
+- 0x04 periodic params (8 bytes)
+  - b0 0x04, b1 0x0e, b2 0x00
+  - b3 magnitude u7 (0..127)
+  - b4 offset = 0, b5 phase = 0
+  - b6..7 period (le16, ms)
+- 0x04 ramp params (9 bytes)
+  - b0 0x04, b1 0x0e
+  - b2..3 start (le16), b4..5 cur_val (le16), b6..7 duration (le16), b8 0x00
+- 0x05 condition params (11 bytes)
+  - set 1 (coeff/saturation): b0 0x05, b1 0x0e, b2 0x00, b3 right_strength, b4 left_strength, b9/b10 subtype values vary by type (spring vs damper/friction)
+  - set 2 (deadband/center): b0 0x05, b1 0x1c, b2 0x00, b3 deadband, b4 center, b9/b10 subtype values
+- 0x40 config (4 bytes)
+  - 0x40 0x04 enable/disable autocenter, 0x40 0x03 set autocenter strength, 0x40 0x11 set range (le16), etc.
+- 0x41 start/stop/clear (4 bytes)
+  - id=0x41, effect_id=0x00 (MUST), command=0x41 START or 0x00 STOP, arg=0x01
+  - Exception: init‑time STOP for autocenter uses effect_id=15
+- 0x42 apply/apply‑like, 0x43 gain
+
+Effect upload and play ordering
+- Constant (FF_CONSTANT)
+  1) 0x02 envelope
+  2) 0x01 main (type=0x00, effect_id=0x00)
+  Play: 0x03 level, then 0x41 START (effect_id=0x00)
+- Periodic (FF_SINE/FF_SQUARE/FF_TRIANGLE/FF_SAW)
+  1) 0x02 envelope
+  2) 0x01 main (type=0x20..0x24, effect_id=0x00)
+  3) 0x04 periodic params (magnitude/period)
+  4) 0x01 main (type repeated, effect_id=0x00)
+  Play: 0x41 START (effect_id=0x00)
+- Condition (FF_SPRING/FF_DAMPER/FF_FRICTION/FF_INERTIA)
+  1) 0x05 coeff/saturation (0x0e)
+  2) 0x05 deadband/center (0x1c)
+  3) 0x01 main (type reflects subkind, effect_id=0x00)
+  Play: 0x41 START (effect_id=0x00)
+- Ramp (FF_RAMP)
+  1) 0x02 envelope
+  2) 0x04 ramp params (device behaves like hold of start level with duration)
+  3) 0x01 main (type=0x24, effect_id=0x00)
+  Play: 0x41 START (effect_id=0x00)
+
+Notes
+- All 0x01 uploads must use EffectID=0x00. This was validated on hardware; using per‑effect IDs causes constant force to fail completely and can break other effects.
+- 16‑bit values are little‑endian (lo byte first).
+- Magnitudes/levels are expected in device ranges (0..127 for periodic magnitude, s8 for constant level). Scaling helpers in code perform conversions from Linux ranges.
+- Autocenter is disabled by zeroing spring coefficients via 0x05 messages and/or via 0x40 commands, then explicitly stopping autocenter (ID 15) during init.
+

src/hid-tmff2.c

@@ -5,8 +5,6 @@
 #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)
@@ -376,15 +374,26 @@ static void tmff2_work_handler(struct work_struct *w)
 
 		effect_delay = state->effect.replay.delay;
 		effect_length = state->effect.replay.length;
+		/* If playing with a finite length, stop when (delay + length) elapses */
 		if (test_bit(FF_EFFECT_PLAYING, &state->flags) && effect_length) {
-			if ((time_now - state->start_time) >=
+			if ((time_now - state->start_time) >= 
 					(effect_delay + effect_length) * state->count) {
 				__clear_bit(FF_EFFECT_PLAYING, &state->flags);
 				__clear_bit(FF_EFFECT_QUEUE_UPDATE, &state->flags);
-
+				/* Request a STOP in process context */
+				__set_bit(FF_EFFECT_QUEUE_STOP, &actions);
 				state->count = 0;
 			}
 		}
+		/* Delay handling for start: only trigger START after replay.delay */
+		if (test_bit(FF_EFFECT_QUEUE_START, &state->flags)) {
+			if ((time_now - state->start_time) >= effect_delay) {
+				__set_bit(FF_EFFECT_QUEUE_START, &actions);
+				__clear_bit(FF_EFFECT_QUEUE_START, &state->flags);
+				/* effect is playing since we're starting it now */
+				__set_bit(FF_EFFECT_PLAYING, &state->flags);
+			} /* else: keep START pending until delay elapsed */
+		}
 
 		if (test_bit(FF_EFFECT_QUEUE_UPLOAD, &state->flags)) {
 			__set_bit(FF_EFFECT_QUEUE_UPLOAD, &actions);
@@ -809,7 +818,6 @@ 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,
@@ -881,44 +889,20 @@ 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);
 
+
+#ifndef TMFF2_DRIVER_VERSION
+#define TMFF2_DRIVER_VERSION "dev"
+#endif
+MODULE_VERSION(TMFF2_DRIVER_VERSION);
+
 MODULE_LICENSE("GPL");

src/hid-tmff2.h

@@ -98,9 +98,6 @@ 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);
 
@@ -119,7 +116,6 @@ 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