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

[ClÃment VUCHENER]

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