Re: [PATCH v3 0/3] new driver for Valve Steam Controller

From: ClÃment VUCHENER
Date: Mon Feb 26 2018 - 06:24:29 EST


2018-02-26 10:50 GMT+01:00 Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>:
> Hi Rodrigo,
>
> On Sun, Feb 25, 2018 at 7:52 PM, Rodrigo Rivas Costa
> <rodrigorivascosta@xxxxxxxxx> wrote:
>> This patchset implements a driver for Valve Steam Controller, based on a
>> reverse analysis by myself.
>
> To me, the code looks OK now. I haven't got the time to do a better
> review, so giving my:
> Acked-by: Bnejamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
>
> However, before we include it, I'd like to have the ACK from
> Pierre-Loup and ClÃment. They should be more qualified than me to say
> if this will interfere with the official Steam client (I think we are
> good now, but that's just my opinion).

I am not qualified to speak about the steam client, it has been a long
time since I have a look at how it uses the controller and I think it
changed a lot.

I checked the code and I think you are not handling wireless
connection correctly. If a wireless controller is already connected
when the driver is loaded, you will not receive a connection event and
forget to register the controller. You can send a 0xb4 request with
empty parameters (0xb4, 0x00, ... ) to force the receiver to send its
connection status (as the type 3 event you are already parsing). Since
user-space program may also send this request, you should make sure
the driver works if it receives a (dis)connected event when it is
already (dis)connected.

>
> Cheers,
> Benjamin
>
>>
>> This is reroll v3, codenamed "lazy lizard". Changes from v2:
>> * Use RCU to do the dynamic connec/disconnect of wireless devices. Please,
>> to anybody that knows their way around RCU, review.
>> * Remove entries in hid-quirks.c as they are no longer needed. This allows
>> this module to be blacklisted without side effects.
>> * Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
>> existing use cases (lizard mode). A user-space tool to do that is
>> linked.
>> * Fully separated axes for joystick and left-pad. As it happens, there are
>> people with too many fingers.
>> * Add fuzz values for left/right pad axes, they are a little wiggly.
>>
>> Notable changes from patchset v1:
>> * Remove references to USB. Now the interesting interfaces are selected by
>> looking for the ones with feature reports.
>> * Feature reports buffers are allocated with hid_alloc_report_buf().
>> * Feature report length is checked, to avoid overflows in case of
>> corrupt/malicius USB devices.
>> * Resolution added to the ABS axes.
>> * A lot of minor cleanups.
>>
>> Rodrigo Rivas Costa (3):
>> HID: add driver for Valve Steam Controller
>> HID: steam: add serial number information.
>> HID: steam: add battery device.
>>
>> drivers/hid/Kconfig | 8 +
>> drivers/hid/Makefile | 1 +
>> drivers/hid/hid-ids.h | 4 +
>> drivers/hid/hid-steam.c | 777 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 790 insertions(+)
>> create mode 100644 drivers/hid/hid-steam.c
>>
>> --
>> 2.16.2
>>