2011-05-28 18:42:56

by Michael Bauer

[permalink] [raw]
Subject: [PATCH] hid: Fix Logitech Driving Force Pro wheel

Hi,

this is a patch for hid-lg.c which fixes the Logitech Driving Force Pro driver.
It was generated against vanilla 2.6.39.

This patch contains two parts:
- Add the quirk "NOGET" to make the wheel work at all in native mode.
- Replace the somehow broken report descriptor with a custom one to have
separate throttle and brake axes.

As there are significant differences in the descriptor (original descriptor
"hides" the separate axes in a 24 bit FF00 usagepage, new descripter replaces
that with two individual 8 bit desktop.y and desktop.rz usages) I provided a
complete replacement descriptor instead trying to patch the original one.
Patching the descriptor seems not feasible as the new one is much larger.

Note: To actually test this you have to use the tool "ltwheelconf" to put the
DFP into it's native mode - See below for more info.


Background:
Most Logitech wheels are initially reporting themselves with a "fallback"
deviceID (USB_DEVICE_ID_LOGITECH_WHEEL - 0xc294), in order to make sure they
are working even without having the proper driver installed.
If the Logitech driver is installed it sends a special command to the wheel
which sets the wheel to "native mode", enabling enhance features like:
- Clutch pedal
- extended wheel rotation range (up to 900 degrees)
- H-gate shifter
- separate axis for throttle / brake
- all buttons
When the wheel is set to native mode it basically disconnects and reconnects
with a different deviceID (USB_DEVICE_ID_LOGITECH_DFP_WHEEL - 0xc298 in this
case).

I am working on a userspace tool [1] which does the switching from fallback to
native mode. During development I found out that the Driving Force Pro wheel
is not supported in native mode - quierk NOGET is missing and the throttle and
brake axes are reported in a combined way only.



Signed-off-by: Michael Bauer <[email protected]>

[1] https://github.com/TripleSpeeder/LTWheelConf

---

--- linux-2.6.39/drivers/hid/hid-lg.c.orig 2011-05-26 22:10:40.099883539
+0200
+++ linux-2.6.39/drivers/hid/hid-lg.c 2011-05-28 20:15:46.368912199 +0200
@@ -41,6 +41,137 @@
#define LG_FF3 0x1000
#define LG_FF4 0x2000

+/* Size of the original descriptor of the Driving Force Pro wheel */
+#define DFP_RDESC_ORIG_SIZE 97
+
+/* Fixed report descriptor for Logitech Driving Force Pro wheel controller
+ *
+ * The original descriptor hides the separate throttle and brake axes in
+ * a custom vendor usage page, providing only a combined value as
+ * GenericDesktop.Y.
+ * This descriptor removes the combined Y axis and instead reports
+ * separate throttle (Y) and brake (RZ).
+ *
+ * This is the original descriptor:
+ *
+ * 0x05, 0x01, // Usage Page (Desktop),
+ * 0x09, 0x04, // Usage (Joystik),
+ * 0xA1, 0x01, // Collection (Application),
+ * 0xA1, 0x02, // Collection (Logical),
+ * 0x95, 0x01, // Report Count (1),
+ * 0x75, 0x0E, // Report Size (14),
+ * 0x15, 0x00, // Logical Minimum (0),
+ * 0x26, 0xFF, 0x3F, // Logical Maximum (16383),
+ * 0x35, 0x00, // Physical Minimum (0),
+ * 0x46, 0xFF, 0x3F, // Physical Maximum (16383),
+ * 0x09, 0x30, // Usage (X),
+ * 0x81, 0x02, // Input (Variable),
+ * 0x95, 0x0E, // Report Count (14),
+ * 0x75, 0x01, // Report Size (1),
+ * 0x25, 0x01, // Logical Maximum (1),
+ * 0x45, 0x01, // Physical Maximum (1),
+ * 0x05, 0x09, // Usage Page (Button),
+ * 0x19, 0x01, // Usage Minimum (01h),
+ * 0x29, 0x0E, // Usage Maximum (0Eh),
+ * 0x81, 0x02, // Input (Variable),
+ * 0x05, 0x01, // Usage Page (Desktop),
+ * 0x95, 0x01, // Report Count (1),
+ * 0x75, 0x04, // Report Size (4),
+ * 0x25, 0x07, // Logical Maximum (7),
+ * 0x46, 0x3B, 0x01, // Physical Maximum (315),
+ * 0x65, 0x14, // Unit (Degrees),
+ * 0x09, 0x39, // Usage (Hat Switch),
+ * 0x81, 0x42, // Input (Variable, Null State),
+ * 0x65, 0x00, // Unit,
+ * 0x95, 0x01, // Report Count (1),
+ * 0x75, 0x08, // Report Size (8),
+ * 0x26, 0xFF, 0x00, // Logical Maximum (255),
+ * 0x46, 0xFF, 0x00, // Physical Maximum (255),
+ * 0x09, 0x31, // Usage (Y),
+ * 0x81, 0x02, // Input (Variable),
+ * 0x06, 0x00, 0xFF, // Usage Page (FF00h),
+ * 0x09, 0x00, // Usage (00h),
+ * 0x95, 0x03, // Report Count (3),
+ * 0x75, 0x08, // Report Size (8),
+ * 0x81, 0x02, // Input (Variable),
+ * 0xC0, // End Collection,
+ * 0xA1, 0x02, // Collection (Logical),
+ * 0x09, 0x02, // Usage (02h),
+ * 0x95, 0x07, // Report Count (7),
+ * 0x91, 0x02, // Output (Variable),
+ * 0xC0, // End Collection,
+ * 0xC0 // End Collection
+ *
+ */
+
+static __u8 dfp_rdesc_fixed[] = {
+0x05, 0x01, /* Usage Page (Desktop), */
+0x09, 0x04, /* Usage (Joystik), */
+0xA1, 0x01, /* Collection (Application), */
+0xA1, 0x02, /* Collection (Logical), */
+0x95, 0x01, /* Report Count (1), */
+0x75, 0x0E, /* Report Size (14), */
+0x14, /* Logical Minimum (0), */
+0x26, 0xFF, 0x3F, /* Logical Maximum (16383), */
+0x34, /* Physical Minimum (0), */
+0x46, 0xFF, 0x3F, /* Physical Maximum (16383), */
+0x09, 0x30, /* Usage (X), */
+0x81, 0x02, /* Input (Variable), */
+0x95, 0x0E, /* Report Count (14), */
+0x75, 0x01, /* Report Size (1), */
+0x25, 0x01, /* Logical Maximum (1), */
+0x45, 0x01, /* Physical Maximum (1), */
+0x05, 0x09, /* Usage Page (Button), */
+0x19, 0x01, /* Usage Minimum (01h), */
+0x29, 0x0E, /* Usage Maximum (0Eh), */
+0x81, 0x02, /* Input (Variable), */
+0x05, 0x01, /* Usage Page (Desktop), */
+0x95, 0x01, /* Report Count (1), */
+0x75, 0x04, /* Report Size (4), */
+0x25, 0x07, /* Logical Maximum (7), */
+0x46, 0x3B, 0x01, /* Physical Maximum (315), */
+0x65, 0x14, /* Unit (Degrees), */
+0x09, 0x39, /* Usage (Hat Switch), */
+0x81, 0x42, /* Input (Variable), */
+0x75, 0x01, /* Report Size (1), */
+0x95, 0x08, /* Report Count (8), */
+0x65, 0x00, /* Unit, */
+0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
+0x25, 0x01, /* Logical Maximum (1), */
+0x45, 0x01, /* Physical Maximum (1), */
+0x09, 0x01, /* Usage (01h), */
+0x81, 0x02, /* Input (Variable), */
+0x05, 0x01, /* Usage Page (Desktop), */
+0x95, 0x01, /* Report Count (1), */
+0x75, 0x08, /* Report Size (8), */
+0x14, /* Logical Minimum (0), */
+0x26, 0xFF, 0x00, /* Logical Maximum (255), */
+0x34, /* Physical Minimum (0), */
+0x46, 0xFF, 0x00, /* Physical Maximum (255), */
+0x09, 0x31, /* Usage (Y), */
+0x81, 0x02, /* Input (Variable), */
+0x95, 0x01, /* Report Count (1), */
+0x75, 0x08, /* Report Size (8), */
+0x14, /* Logical Minimum (0), */
+0x26, 0xFF, 0x00, /* Logical Maximum (255), */
+0x34, /* Physical Minimum (0), */
+0x46, 0xFF, 0x00, /* Physical Maximum (255), */
+0x09, 0x35, /* Usage (Rz), */
+0x81, 0x02, /* Input (Variable), */
+0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
+0x95, 0x01, /* Report Count (1), */
+0x75, 0x08, /* Report Size (8), */
+0x09, 0x00, /* Usage (00h), */
+0x81, 0x02, /* Input (Variable), */
+0xC0, /* End Collection, */
+0xA1, 0x02, /* Collection (Logical), */
+0x09, 0x02, /* Usage (02h), */
+0x95, 0x07, /* Report Count (7), */
+0x91, 0x02, /* Output (Variable), */
+0xC0, /* End Collection, */
+0xC0 /* End Collection */
+};
+
/*
* Certain Logitech keyboards send in report #3 keys which are far
* above the logical maximum described in descriptor. This extends
@@ -74,6 +205,18 @@ static __u8 *lg_report_fixup(struct hid_
rdesc[47] = 0x95;
rdesc[48] = 0x0B;
}
+
+ switch (hdev->product) {
+ case USB_DEVICE_ID_LOGITECH_DFP_WHEEL:
+ if (*rsize == DFP_RDESC_ORIG_SIZE) {
+ hid_info(hdev,
+ "fixing up Logitech Driving Force Pro report
descriptor\n");
+ rdesc = dfp_rdesc_fixed;
+ *rsize = sizeof(dfp_rdesc_fixed);
+ }
+ break;
+ }
+
return rdesc;
}

@@ -378,7 +521,7 @@ static const struct hid_device_id lg_dev
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
USB_DEVICE_ID_LOGITECH_G25_WHEEL),
.driver_data = LG_FF },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
USB_DEVICE_ID_LOGITECH_DFP_WHEEL),
- .driver_data = LG_FF },
+ .driver_data = LG_NOGET | LG_FF },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
USB_DEVICE_ID_LOGITECH_WII_WHEEL),
.driver_data = LG_FF4 },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
USB_DEVICE_ID_LOGITECH_WINGMAN_FFG ),


2011-05-28 21:32:04

by Simon Wood

[permalink] [raw]
Subject: Re: [PATCH] hid: Fix Logitech Driving Force Pro wheel

Hi Michael and all,

> As there are significant differences in the descriptor (original
> descriptor
> "hides" the separate axes in a 24 bit FF00 usagepage, new descripter
> replaces
> that with two individual 8 bit desktop.y and desktop.rz usages) I provided
> a
> complete replacement descriptor instead trying to patch the original one.
> Patching the descriptor seems not feasible as the new one is much larger.

I think there a couple problems with your replacement descriptor, although
I don't have a wheel to test with).

1). There is no 'NULL state' on the hatswitch
2). The original does Y then 3 others, the replacement does Y + RZ then 1
other.

When I was looking at doing a similar thing for another wheel I recall
that there was a way of doing a 1 byte NOP, but can't remember how at the
moment.

So you might want to try just patching the original as follows.

> + * 0x95, 0x01, // Report Count (1),
> + * 0x75, 0x08, // Report Size (8),
> + * 0x26, 0xFF, 0x00, // Logical Maximum (255),
> + * 0x46, 0xFF, 0x00, // Physical Maximum (255),
> + * 0x09, 0x31, // Usage (Y),
> + * 0x81, 0x02, // Input (Variable),
> + * 0x06, 0x00, 0xFF, // Usage Page (FF00h), -> NOP +
ReportCount(3)
> + * 0x09, 0x00, // Usage (00h), -> usage(Z)
> + * 0x95, 0x03, // Report Count (3), -> usage(Rx)
> + * 0x75, 0x08, // Report Size (8), -> usage(Ry)
> + * 0x81, 0x02, // Input (Variable),
> + * 0xC0, // End Collection,

Most settings roll over between sections, so don't need to be redefined. I
don't think the exact 'usage()' is important as Linux will just see them
as axis.

Cheers,
Simon

2011-05-28 23:47:36

by Simon Wood

[permalink] [raw]
Subject: Re: [PATCH] hid: Fix Logitech Driving Force Pro wheel


> 2). The original does Y then 3 others, the replacement does Y + RZ then 1
> other.

OK I missed that you had 'disabled' the first byte value after the hat
switch (which is presumably the combined acc-brake), in preference for the
seperate ACC and Brake.

Is there a reason to prevent the combined acc-brake being reported to the
system?

Surely it would be best to report all options so that the app can decide
which one to use.... I can see that you might want to relabel them for
consistance with Windows.

Do you know what the 4th (and last) byte value is?
Simon

2011-05-29 06:46:52

by Michael Bauer

[permalink] [raw]
Subject: Re: [PATCH] hid: Fix Logitech Driving Force Pro wheel

Hi Simon,

> > 2). The original does Y then 3 others, the replacement does Y + RZ then 1
> > other.
>
> OK I missed that you had 'disabled' the first byte value after the hat
> switch (which is presumably the combined acc-brake), in preference for the
> seperate ACC and Brake.
>
> Is there a reason to prevent the combined acc-brake being reported to the
> system?
>
> Surely it would be best to report all options so that the app can decide
> which one to use.... I can see that you might want to relabel them for
> consistance with Windows.

Well, i thought about this also and normally i would agree with your
statement. But my conclusion was to remove the combined axis. Looking at
Windows all wheel controllers i saw so far have a setting in their driver to
enable or disable the separate axis. But i never saw a controller which
provided both separate and combined at the same time.
I am pretty sure there will be trouble when assigning these controls within an
application.
Often the control assignement works like this: User clicks "set axis for
acceleration", application asks "please move the desired axis" and then
automatically choses the "moving" axis.
If we report both combined and separate at the same time, the application will
have a hard time deciding which one to use, as they both will change their
values.

> Do you know what the 4th (and last) byte value is?
> Simon

No idea - But I never observed any change being reported on this byte, so at
the moment i assume it is completely unused.

Best regards
Michael

2011-05-29 14:50:39

by Simon Wood

[permalink] [raw]
Subject: Re: [PATCH] hid: Fix Logitech Driving Force Pro wheel


> Often the control assignement works like this: User clicks "set axis for
> acceleration", application asks "please move the desired axis" and then
> automatically choses the "moving" axis.
> If we report both combined and separate at the same time, the application
> will
> have a hard time deciding which one to use, as they both will change their
> values.
>

Fair enough, I played a little trying to massage the original descriptor
but could not find a sensible solution - so I guess we'll have to provide
a full replacement.

I don't see the need to comment the original/replacement blocks, so
probably a simple 'hex block' would be good.

Regarding the replacement, you appear to have a lot of unnecessary code
there. You could try with something like:
--
...
+0x09, 0x39, /* Usage (Hat Switch), */
+0x81, 0x42, /* Input (Variable), */
+0x75, 0x08, /* Report Size (8), */
+0x95, 0x08, /* Report Count (1), */
+0x65, 0x00, /* Unit, */
+0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
+0x26, 0xFF, 0x00, /* Logical Maximum (255), */
+0x46, 0xFF, 0x00, /* Physical Maximum (255), */
+0x09, 0x01, /* Usage (01h), */
+0x81, 0x02, /* Input (Variable), */
+0xA4, /* Push, */
+0x05, 0x01, /* Usage Page (Desktop), */
+0x95, 0x01, /* Report Count (2), */
+0x09, 0x31, /* Usage (Y), */
+0x09, 0x35, /* Usage (Rz), */
+0x81, 0x02, /* Input (Variable), */
+0xB4, /* Pop, */
+0x81, 0x02, /* Input (Variable), */
+0xC0, /* End Collection, */
...
--

Cheers,
Simon.

2011-05-30 06:17:08

by Michael Bauer

[permalink] [raw]
Subject: Re: [PATCH] hid: Fix Logitech Driving Force Pro wheel

Hi all,

> Fair enough, I played a little trying to massage the original descriptor
> but could not find a sensible solution - so I guess we'll have to provide
> a full replacement.
Yes, i also spent some time but it does not seem to be possible in a sensible
way.

> I don't see the need to comment the original/replacement blocks, so
> probably a simple 'hex block' would be good.
Fine for me, i will remove it in the next patch.

> Regarding the replacement, you appear to have a lot of unnecessary code
> there. You could try with something like:
> --
> ...
> +0x09, 0x39, /* Usage (Hat Switch), */
> +0x81, 0x42, /* Input (Variable), */
> +0x75, 0x08, /* Report Size (8), */
> +0x95, 0x08, /* Report Count (1), */
> +0x65, 0x00, /* Unit, */
> +0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
> +0x26, 0xFF, 0x00, /* Logical Maximum (255), */
> +0x46, 0xFF, 0x00, /* Physical Maximum (255), */
> +0x09, 0x01, /* Usage (01h), */
> +0x81, 0x02, /* Input (Variable), */
> +0xA4, /* Push, */
> +0x05, 0x01, /* Usage Page (Desktop), */
> +0x95, 0x01, /* Report Count (2), */
> +0x09, 0x31, /* Usage (Y), */
> +0x09, 0x35, /* Usage (Rz), */
> +0x81, 0x02, /* Input (Variable), */
> +0xB4, /* Pop, */
> +0x81, 0x02, /* Input (Variable), */
> +0xC0, /* End Collection, */
> ...
> --

Good point - didn't think of the Push/Pop commands...

I will create an updated patch and submit it here again.

Thanks and regards
Michael