public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hid: Fix Logitech Driving Force Pro wheel
@ 2011-05-28 18:37 Michael Bauer
  2011-05-28 21:31 ` simon
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Bauer @ 2011-05-28 18:37 UTC (permalink / raw)
  To: linux-input; +Cc: linux-kernel, Jiri Kosina

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 <michael@m-bauer.org>

[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 ),

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] hid: Fix Logitech Driving Force Pro wheel
  2011-05-28 18:37 [PATCH] hid: Fix Logitech Driving Force Pro wheel Michael Bauer
@ 2011-05-28 21:31 ` simon
  2011-05-28 23:47   ` simon
  0 siblings, 1 reply; 6+ messages in thread
From: simon @ 2011-05-28 21:31 UTC (permalink / raw)
  To: Michael Bauer; +Cc: linux-input, linux-kernel, Jiri Kosina

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] hid: Fix Logitech Driving Force Pro wheel
  2011-05-28 21:31 ` simon
@ 2011-05-28 23:47   ` simon
  2011-05-29  6:46     ` Michael Bauer
  0 siblings, 1 reply; 6+ messages in thread
From: simon @ 2011-05-28 23:47 UTC (permalink / raw)
  To: Michael Bauer; +Cc: linux-input, linux-kernel, Jiri Kosina


> 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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] hid: Fix Logitech Driving Force Pro wheel
  2011-05-28 23:47   ` simon
@ 2011-05-29  6:46     ` Michael Bauer
  2011-05-29 14:50       ` simon
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Bauer @ 2011-05-29  6:46 UTC (permalink / raw)
  To: linux-input; +Cc: simon, linux-kernel, Jiri Kosina

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] hid: Fix Logitech Driving Force Pro wheel
  2011-05-29  6:46     ` Michael Bauer
@ 2011-05-29 14:50       ` simon
  2011-05-30  6:17         ` Michael Bauer
  0 siblings, 1 reply; 6+ messages in thread
From: simon @ 2011-05-29 14:50 UTC (permalink / raw)
  To: Michael Bauer; +Cc: linux-input, simon, linux-kernel, Jiri Kosina


> 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.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] hid: Fix Logitech Driving Force Pro wheel
  2011-05-29 14:50       ` simon
@ 2011-05-30  6:17         ` Michael Bauer
  0 siblings, 0 replies; 6+ messages in thread
From: Michael Bauer @ 2011-05-30  6:17 UTC (permalink / raw)
  To: linux-input; +Cc: simon, linux-kernel, Jiri Kosina

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-05-30  6:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-28 18:37 [PATCH] hid: Fix Logitech Driving Force Pro wheel Michael Bauer
2011-05-28 21:31 ` simon
2011-05-28 23:47   ` simon
2011-05-29  6:46     ` Michael Bauer
2011-05-29 14:50       ` simon
2011-05-30  6:17         ` Michael Bauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox