Bug 204191

If it helps, the driver is treating the wheel as a generic HID, because hidpp_validate_device returns false, because hidpp_validate_report(... REPORT_ID_HIDPP_SHORT, ...) returns false, because report_length is false.

Confirming the same issue. Good work with bisecting it.

I can confirm and reproduce the issue as well.

dmesg logs with 5.2.10 and 4.19.69lts kernel

https://pastebin.com/U144bTar

Has there been any progress on this?  5.2.13, 5.2.14, and 5.3 are all released now.  This has completely broken use of the Logitech steering wheel with driving/racing simulation.

Created attachment 285039 [details]
USB HID descriptor parsed

I did some more digging and found that the USB HID descriptor for the wheel has a report ID for REPORT_ID_HIDPP_LONG (line 56) and REPORT_ID_HIDPP_VERY_LONG (line 69), but no report ID for REPORT_ID_HIDPP_SHORT, which is why hidpp_validate_report(hdev, REPORT_ID_HIDPP_SHORT, ...) fails.

Created attachment 285041 [details]
USB HID descriptor binary

I am looking at the HID version 1.11 spec, but I am not familiar with it, having trouble finding my way around.
Does this seem to be a mistake in the device implementation that needs to be worked around in the driver, or a mistaken assumption in the driver that all three  REPORT_ID_HIDPP variations will always be present?
Anyone know which section of the spec would be relevant to this?  The report examples in the spec just show one section, so I don't understand how the LONG, VERY_LONG, and SHORT reports come into this.

The REPORT_ID_HIDPP_* reports are from the HID++ spec. There's a couple of links on it in the readme of this repo https://github.com/cvuchener/hidpp

(In reply to Sam Bazley from comment #9)
> The REPORT_ID_HIDPP_* reports are from the HID++ spec. There's a couple of
> links on it in the readme of this repo https://github.com/cvuchener/hidpp

So this is a Logitech only problem, not a generic USB HID problem?  
I don't understand why this change is not reverted yet, it is a change for a logitech specific protocol and it breaks logitech controllers, or at least logitech steering wheel controllers.  
I have not pulled out my force feedback joystick recently, anyone know if those are broken as well?  Does this change just make a fancy mouse work better or something? 
I thought the kernel approval process was pretty strict about not accepting changes which break working applications.  Seems a clear case of "breaks every driving simulation game" so would get this change reverted more quickly than the two and a half months this bug report has been active.

Created attachment 285321 [details]
Fix detection of Logitech G920 as hidpp device

Apart from the missing REPORT_ID_HIDPP_SHORT which causes hidpp_validate_report() to fail there is another problem in the driver.

Supposing that the check passes there will be a null pointer dereference in g920_get_config() -> hidpp_ff_init() line 3 of this function:

static int hidpp_ff_init(struct hidpp_device *hidpp, u8 feature_index)
{
	struct hid_device *hid = hidpp->hid_dev;
        struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
        struct input_dev *dev = hidinput->input;
        ...

This is caused by the way hid_hw_start(hdev, 0) is called before g920_get_config() gets called. If I call hid_hw_start(hdev, HID_CONNECT_DEFAULT) instead there is no problem. This is actually the way it was called before when it still did work.

I have attached a patch that fixes the two problems in hid-logitech-hidpp.c. While the second part is specific to the G920 the first part with the REPORT_IDs is not and I don't know if this could brake other things.

Created attachment 285323 [details]
Fix detection of Logitech G920 as hidpp device

Sorry, the previous patch was a little premature. There seems to be something more special about the G920 (device must not be closed/stopped before returning from hidpp_probe() ?). The patch now simply restores the previously working behaviour.

Thank you. This patch works wonderfully. Applied it to kernel 5.3.2 and forcefeedback on my G920 started working again.

Created attachment 285375 [details]
G920 fixes patch 1

AFAICT, there were actually two regression in G920 support since 4.19. I submitted the following three patches upstream to fix them:

https://lore.kernel.org/lkml/20191007051240.4410-1-andrew.smirnov@gmail.com/

Created attachment 285377 [details]
G920 fixes patch 2

Created attachment 285379 [details]
G920 fixes patch 3

Tried the 3-patch solution with the Fedora build of 5.2.18 and seems to work correctly with my G920.
Someone at Steam mentioned that one of their kernel engineers was working on this. Is that Andrey?  If not someone should make sure they are aware of this thread, looks like the solution is good, just needs to get reviewed and accepted upstream (and let the distributions know they should backport).
I think I will submit a Fedora request to have this backported, unless there is any reason to wait.

I tested the patches earlier and it seems to work properly now. Thanks Andrey :·)

Fedora kernel dev said that Fedora would prefer to wait until this had been ack'd by upstream maintainer before including.  If possible please add a reference to any LMKL confirmation that these patches will be accepted upstream, hopefully that will be enough for Fedora to include(the idea was include a patch that could be dropped as unneeded in the future, and not have to carry these patches forever).
thanks

@Chris Caudle

Yes, that would be me, I was asked to look at this and provided the necessary HW by the fine folks at Valve. I was aware of this thread from the discussion on Steam, although forgot to refresh this page and did not see the patch @mikkle attached (or any of their messages) before I posted my version of the fixes.

FWIW, I tagged all of my patches to be included into stable trees, so, once Greg KH picks it up, I think Fedora (which is what I personally use as well), Ubuntu or any other distribution should get them that way eventually. I'll keep this ticket updated on the status of my submission regardless.

@Sam Bazely
Good to hear! Since you are CC'd on that thread, feel free to reply to cover letter and provide a "Tested-By" tag.

Patches work for me, too. Actually Andrey's patches do some more things, i.e. auto-centering when e.g. resetting car in Dirt Rally never worked before and now it does.

Sorry for Bump on this, but its been quite some time now. Is this finally landed upstream or we still need to patch the kernel?

This is been a problem at least 3 kernel cycles already

5.3.9 has the patch. If you're having issues using the wheel with Steam, you might need to run `steam --reset`, although that will reset other settings as well, so you should probably test with `jstest(-gtk)`

(In reply to Sam Bazley from comment #23)
> 5.3.9 has the patch. If you're having issues using the wheel with Steam, you
> might need to run `steam --reset`, although that will reset other settings
> as well, so you should probably test with `jstest(-gtk)`

Thanks. I have 5.4.2 patched with Andrey Smirnov patches. And it works, i was just wondering if its already upstreamed and i can discard the patches now.

if its in 5.3.9 thats nice to hear, i guess its in 5.4+ also then?

(In reply to Xpander from comment #24)
> i guess its in 5.4+ also then?

Yes