Re: [PATCH 1/3] HID: add driver for Valve Steam Controller

2018-02-21 Thread Pierre-Loup A. Griffais 

On 02/14/2018 03:29 PM, Rodrigo Rivas Costa wrote:

On Wed, Feb 14, 2018 at 03:45:14PM +0100, Benjamin Tissoires wrote:

I think I had a look at this a while ago, and didn't want to interfere
with SteamOS regarding this. I think your patch should be fine in that
regard, but have you tried SteamOS on a kernel patched with your
series? Does it behave properly or will it break?


Well, SteamOS is just a modified Debian with the Steam Client running in
Big Picture mode (fullscreen). I tried the Steam Client with this driver
and it works just fine:
- The first thing the Steam Client (SC) does is to disable the virtual
   keyboard and mouse, so not creating those input devices make no
   difference.


There's a bit more to it than that; for example, on SteamOS, the virtual 
keyboard/mouse will be re-enabled when switching to Desktop Mode.


In addition, chord configurations can include a bindable action that 
temporarily programs the controller to re-enable the virtual 
keyboard/mouse (what we call "Lizard Mode"). People use this to navigate 
around cases where our software input doesn't cut it, like a launcher 
pop-up window that Steam isn't able to hook into for whatever reason.


That behavior will also change over time since the client is often 
updated with support for new controller features.


Thanks,
 - Pierre-Loup


- Input events are sent both to the libusb (or whatever SC uses) and
   this driver.
- The only source of conflict would be in sending commands to the
   controller. But currently the only command sent is the get_serial, and
   that seems to do no harm.


+
+   hid_info(hdev, "Steam Controller connected");
+
+   input = input_allocate_device();


Don't you need one input node per wireless gamepad too?


No, the wired and wireless methods are two different USB devices.
The wireless adaptor is a small usb device that creates 4 HID interfaces
for the 4 to-be-connected controllers. When the 'connected' event
arrives on one of those interfaces we will create one input device.
Another controller connected will use a different interface so another
unrelated input device will be created.

The wired adaptor is the controller connected directly to USB: it
creates one HID inteface and one input device directly.

Anyway, one steam_device will contain 0 or 1 input_device.


+   input->name = "Steam Controller";


In case of the wireless controllers, you might want to personalize this a bit.


Do you mean e.g. "Wireless Steam Controller"? Would be wise to add the serial
number here (from PATCH 2/3?)? Or is the 'uniq' enough for that?


+   input_set_abs_params(input, ABS_Z, 0, 255, 0, 0);
+   input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0);
+   input_set_abs_params(input, ABS_X, -32767, 32767, 0, 0);
+   input_set_abs_params(input, ABS_Y, -32767, 32767, 0, 0);


You are also probably missing the resolution bits. We need to
accurately report the physical dimensions to the user space (thanks to
the resolution)


+   input_set_abs_params(input, ABS_RX, -32767, 32767, 0, 0);
+   input_set_abs_params(input, ABS_RY, -32767, 32767, 0, 0);


What do RX/RY correspond to?


This gamepad has 1 joystick and 2 touchpads (lpad and rpad).
I'm mapping the joystick/lpad to ABS_X/ABS_Y and the rpad to
ABS_RX/ABS_RY.

About the resolution, I looked around other drivers and many have 0 as
resolution... is it necessary only in the pads or the joystick too?
And what are the units of that value?
For reference, the pads are round and exactly 40 mm in diameter.

Why the lpad and the joystick are mapped to the same axes? Well, because
the device returns them at the same offset. We could make them
apart by using bits 10.3 (lpad_touch) and 10.7 (lpad_and_joy) but I
don't think it is worth it... you use the joystick and the lpad with the
same thumb!


Anyway. Thanks for the driver, there are a few bits to fix, nothing
scary though.


Great, I'll correct all those issues and resubmit the patches in a few
days. It is my very first driver and I'm still a bit slow...

Thank you for your attention!
Rodrigo.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] HID: add driver for Valve Steam Controller

2018-02-21 Thread Pierre-Loup A. Griffais 

On 02/14/2018 03:29 PM, Rodrigo Rivas Costa wrote:

On Wed, Feb 14, 2018 at 03:45:14PM +0100, Benjamin Tissoires wrote:

I think I had a look at this a while ago, and didn't want to interfere
with SteamOS regarding this. I think your patch should be fine in that
regard, but have you tried SteamOS on a kernel patched with your
series? Does it behave properly or will it break?


Well, SteamOS is just a modified Debian with the Steam Client running in
Big Picture mode (fullscreen). I tried the Steam Client with this driver
and it works just fine:
- The first thing the Steam Client (SC) does is to disable the virtual
   keyboard and mouse, so not creating those input devices make no
   difference.


There's a bit more to it than that; for example, on SteamOS, the virtual 
keyboard/mouse will be re-enabled when switching to Desktop Mode.


In addition, chord configurations can include a bindable action that 
temporarily programs the controller to re-enable the virtual 
keyboard/mouse (what we call "Lizard Mode"). People use this to navigate 
around cases where our software input doesn't cut it, like a launcher 
pop-up window that Steam isn't able to hook into for whatever reason.


That behavior will also change over time since the client is often 
updated with support for new controller features.


Thanks,
 - Pierre-Loup


- Input events are sent both to the libusb (or whatever SC uses) and
   this driver.
- The only source of conflict would be in sending commands to the
   controller. But currently the only command sent is the get_serial, and
   that seems to do no harm.


+
+   hid_info(hdev, "Steam Controller connected");
+
+   input = input_allocate_device();


Don't you need one input node per wireless gamepad too?


No, the wired and wireless methods are two different USB devices.
The wireless adaptor is a small usb device that creates 4 HID interfaces
for the 4 to-be-connected controllers. When the 'connected' event
arrives on one of those interfaces we will create one input device.
Another controller connected will use a different interface so another
unrelated input device will be created.

The wired adaptor is the controller connected directly to USB: it
creates one HID inteface and one input device directly.

Anyway, one steam_device will contain 0 or 1 input_device.


+   input->name = "Steam Controller";


In case of the wireless controllers, you might want to personalize this a bit.


Do you mean e.g. "Wireless Steam Controller"? Would be wise to add the serial
number here (from PATCH 2/3?)? Or is the 'uniq' enough for that?


+   input_set_abs_params(input, ABS_Z, 0, 255, 0, 0);
+   input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0);
+   input_set_abs_params(input, ABS_X, -32767, 32767, 0, 0);
+   input_set_abs_params(input, ABS_Y, -32767, 32767, 0, 0);


You are also probably missing the resolution bits. We need to
accurately report the physical dimensions to the user space (thanks to
the resolution)


+   input_set_abs_params(input, ABS_RX, -32767, 32767, 0, 0);
+   input_set_abs_params(input, ABS_RY, -32767, 32767, 0, 0);


What do RX/RY correspond to?


This gamepad has 1 joystick and 2 touchpads (lpad and rpad).
I'm mapping the joystick/lpad to ABS_X/ABS_Y and the rpad to
ABS_RX/ABS_RY.

About the resolution, I looked around other drivers and many have 0 as
resolution... is it necessary only in the pads or the joystick too?
And what are the units of that value?
For reference, the pads are round and exactly 40 mm in diameter.

Why the lpad and the joystick are mapped to the same axes? Well, because
the device returns them at the same offset. We could make them
apart by using bits 10.3 (lpad_touch) and 10.7 (lpad_and_joy) but I
don't think it is worth it... you use the joystick and the lpad with the
same thumb!


Anyway. Thanks for the driver, there are a few bits to fix, nothing
scary though.


Great, I'll correct all those issues and resubmit the patches in a few
days. It is my very first driver and I'm still a bit slow...

Thank you for your attention!
Rodrigo.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] HID: add driver for Valve Steam Controller

2018-02-14 Thread Rodrigo Rivas Costa 
On Wed, Feb 14, 2018 at 03:45:14PM +0100, Benjamin Tissoires wrote:
> I think I had a look at this a while ago, and didn't want to interfere
> with SteamOS regarding this. I think your patch should be fine in that
> regard, but have you tried SteamOS on a kernel patched with your
> series? Does it behave properly or will it break?

Well, SteamOS is just a modified Debian with the Steam Client running in
Big Picture mode (fullscreen). I tried the Steam Client with this driver
and it works just fine: 
- The first thing the Steam Client (SC) does is to disable the virtual
  keyboard and mouse, so not creating those input devices make no
  difference.
- Input events are sent both to the libusb (or whatever SC uses) and
  this driver.
- The only source of conflict would be in sending commands to the
  controller. But currently the only command sent is the get_serial, and
  that seems to do no harm.

> > +
> > +   hid_info(hdev, "Steam Controller connected");
> > +
> > +   input = input_allocate_device();
> 
> Don't you need one input node per wireless gamepad too?
> 
No, the wired and wireless methods are two different USB devices.
The wireless adaptor is a small usb device that creates 4 HID interfaces
for the 4 to-be-connected controllers. When the 'connected' event
arrives on one of those interfaces we will create one input device.
Another controller connected will use a different interface so another
unrelated input device will be created.

The wired adaptor is the controller connected directly to USB: it
creates one HID inteface and one input device directly.

Anyway, one steam_device will contain 0 or 1 input_device.

> > +   input->name = "Steam Controller";
> 
> In case of the wireless controllers, you might want to personalize this a bit.

Do you mean e.g. "Wireless Steam Controller"? Would be wise to add the serial
number here (from PATCH 2/3?)? Or is the 'uniq' enough for that?

> > +   input_set_abs_params(input, ABS_Z, 0, 255, 0, 0);
> > +   input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0);
> > +   input_set_abs_params(input, ABS_X, -32767, 32767, 0, 0);
> > +   input_set_abs_params(input, ABS_Y, -32767, 32767, 0, 0);
> 
> You are also probably missing the resolution bits. We need to
> accurately report the physical dimensions to the user space (thanks to
> the resolution)
> 
> > +   input_set_abs_params(input, ABS_RX, -32767, 32767, 0, 0);
> > +   input_set_abs_params(input, ABS_RY, -32767, 32767, 0, 0);
> 
> What do RX/RY correspond to?

This gamepad has 1 joystick and 2 touchpads (lpad and rpad).
I'm mapping the joystick/lpad to ABS_X/ABS_Y and the rpad to
ABS_RX/ABS_RY.

About the resolution, I looked around other drivers and many have 0 as
resolution... is it necessary only in the pads or the joystick too?
And what are the units of that value?
For reference, the pads are round and exactly 40 mm in diameter.

Why the lpad and the joystick are mapped to the same axes? Well, because
the device returns them at the same offset. We could make them
apart by using bits 10.3 (lpad_touch) and 10.7 (lpad_and_joy) but I
don't think it is worth it... you use the joystick and the lpad with the
same thumb!

> Anyway. Thanks for the driver, there are a few bits to fix, nothing
> scary though.

Great, I'll correct all those issues and resubmit the patches in a few
days. It is my very first driver and I'm still a bit slow...

Thank you for your attention!
Rodrigo.

Re: [PATCH 1/3] HID: add driver for Valve Steam Controller

2018-02-14 Thread Rodrigo Rivas Costa 
On Wed, Feb 14, 2018 at 03:45:14PM +0100, Benjamin Tissoires wrote:
> I think I had a look at this a while ago, and didn't want to interfere
> with SteamOS regarding this. I think your patch should be fine in that
> regard, but have you tried SteamOS on a kernel patched with your
> series? Does it behave properly or will it break?

Well, SteamOS is just a modified Debian with the Steam Client running in
Big Picture mode (fullscreen). I tried the Steam Client with this driver
and it works just fine: 
- The first thing the Steam Client (SC) does is to disable the virtual
  keyboard and mouse, so not creating those input devices make no
  difference.
- Input events are sent both to the libusb (or whatever SC uses) and
  this driver.
- The only source of conflict would be in sending commands to the
  controller. But currently the only command sent is the get_serial, and
  that seems to do no harm.

> > +
> > +   hid_info(hdev, "Steam Controller connected");
> > +
> > +   input = input_allocate_device();
> 
> Don't you need one input node per wireless gamepad too?
> 
No, the wired and wireless methods are two different USB devices.
The wireless adaptor is a small usb device that creates 4 HID interfaces
for the 4 to-be-connected controllers. When the 'connected' event
arrives on one of those interfaces we will create one input device.
Another controller connected will use a different interface so another
unrelated input device will be created.

The wired adaptor is the controller connected directly to USB: it
creates one HID inteface and one input device directly.

Anyway, one steam_device will contain 0 or 1 input_device.

> > +   input->name = "Steam Controller";
> 
> In case of the wireless controllers, you might want to personalize this a bit.

Do you mean e.g. "Wireless Steam Controller"? Would be wise to add the serial
number here (from PATCH 2/3?)? Or is the 'uniq' enough for that?

> > +   input_set_abs_params(input, ABS_Z, 0, 255, 0, 0);
> > +   input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0);
> > +   input_set_abs_params(input, ABS_X, -32767, 32767, 0, 0);
> > +   input_set_abs_params(input, ABS_Y, -32767, 32767, 0, 0);
> 
> You are also probably missing the resolution bits. We need to
> accurately report the physical dimensions to the user space (thanks to
> the resolution)
> 
> > +   input_set_abs_params(input, ABS_RX, -32767, 32767, 0, 0);
> > +   input_set_abs_params(input, ABS_RY, -32767, 32767, 0, 0);
> 
> What do RX/RY correspond to?

This gamepad has 1 joystick and 2 touchpads (lpad and rpad).
I'm mapping the joystick/lpad to ABS_X/ABS_Y and the rpad to
ABS_RX/ABS_RY.

About the resolution, I looked around other drivers and many have 0 as
resolution... is it necessary only in the pads or the joystick too?
And what are the units of that value?
For reference, the pads are round and exactly 40 mm in diameter.

Why the lpad and the joystick are mapped to the same axes? Well, because
the device returns them at the same offset. We could make them
apart by using bits 10.3 (lpad_touch) and 10.7 (lpad_and_joy) but I
don't think it is worth it... you use the joystick and the lpad with the
same thumb!

> Anyway. Thanks for the driver, there are a few bits to fix, nothing
> scary though.

Great, I'll correct all those issues and resubmit the patches in a few
days. It is my very first driver and I'm still a bit slow...

Thank you for your attention!
Rodrigo.

Re: [PATCH 1/3] HID: add driver for Valve Steam Controller

2018-02-14 Thread Philippe Ombredanne 
Benjamin, Rodrigo,

On Wed, Feb 14, 2018 at 3:45 PM, Benjamin Tissoires
 wrote:
> On Tue, Feb 13, 2018 at 1:03 PM, Rodrigo Rivas Costa 
>  wrote:


>> --- /dev/null
>> +++ b/drivers/hid/hid-steam.c
>> @@ -0,0 +1,480 @@
>> +// SPDX-License-Identifier: GPL-2.0
>
> Non standard header

Benjamin:
What do you mean by this?
This is following the proper style for this line as documented (and
discussed on list at great length) [1]

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/license-rules.rst


>> +/*
>> + * HID driver for Valve Steam Controller
>> + *
>> + * Supports both the wired and wireless interfaces.
>> + *
>> + * Copyright (c) 2018 Rodrigo Rivas Costa 
>> + */
>> +
>> +/*
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by the 
>> Free
>> + * Software Foundation; either version 2 of the License, or (at your option)
>> + * any later version.
>> + */

Rodrigo,
Since you used the proper SPDX tag (in the proper style as explained
in the doc), you can remove this boilerplate alright as it does double
duty with the tag.

-- 
Cordially
Philippe Ombredanne

Re: [PATCH 1/3] HID: add driver for Valve Steam Controller

2018-02-14 Thread Philippe Ombredanne 
Benjamin, Rodrigo,

On Wed, Feb 14, 2018 at 3:45 PM, Benjamin Tissoires
 wrote:
> On Tue, Feb 13, 2018 at 1:03 PM, Rodrigo Rivas Costa 
>  wrote:


>> --- /dev/null
>> +++ b/drivers/hid/hid-steam.c
>> @@ -0,0 +1,480 @@
>> +// SPDX-License-Identifier: GPL-2.0
>
> Non standard header

Benjamin:
What do you mean by this?
This is following the proper style for this line as documented (and
discussed on list at great length) [1]

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/license-rules.rst


>> +/*
>> + * HID driver for Valve Steam Controller
>> + *
>> + * Supports both the wired and wireless interfaces.
>> + *
>> + * Copyright (c) 2018 Rodrigo Rivas Costa 
>> + */
>> +
>> +/*
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by the 
>> Free
>> + * Software Foundation; either version 2 of the License, or (at your option)
>> + * any later version.
>> + */

Rodrigo,
Since you used the proper SPDX tag (in the proper style as explained
in the doc), you can remove this boilerplate alright as it does double
duty with the tag.

-- 
Cordially
Philippe Ombredanne

Re: [PATCH 1/3] HID: add driver for Valve Steam Controller

2018-02-14 Thread Benjamin Tissoires 
Hi Rodrigo,

On Tue, Feb 13, 2018 at 1:03 PM, Rodrigo Rivas Costa
 wrote:
> There are two ways to connect the Steam Controller: directly to the USB
> or with the USB wireless adapter.  Both methods are similar, but the
> wireless adapter can connect up to 4 devices at the same time.
>
> The wired device will appear as 3 interfaces: a virtual mouse, a virtual
> keyboard and a custom HID device.
>
> The wireless device will appear as 5 interfaces: a virtual keyboard and
> 4 custom HID devices, that will remain silent until a device is actually
> connected.
>
> The custom HID device has a report descriptor with all vendor specific
> usages, so the hid-generic is not very useful. In a PC/SteamBox Valve
> Steam Client provices a software translation by using direct USB access
> and a creates a uinput virtual gamepad.

I think I had a look at this a while ago, and didn't want to interfere
with SteamOS regarding this. I think your patch should be fine in that
regard, but have you tried SteamOS on a kernel patched with your
series? Does it behave properly or will it break?

See more comments inlined.

>
> This driver was reverse engineered to provide direct kernel support in
> case you cannot, or do not want to, use Valve Steam Client. It disables
> the virtual keyboard and mouse, as they are not so useful when you have
> a working gamepad.
>
> Working: buttons, axes, pads, wireless connect/disconnect.
>
> TO-DO: Battery, force-feedback, accelerometer/gyro, led, beeper...
>
> Signed-off-by: Rodrigo Rivas Costa 
> ---
>  drivers/hid/Kconfig  |   8 +
>  drivers/hid/Makefile |   1 +
>  drivers/hid/hid-ids.h|   4 +
>  drivers/hid/hid-quirks.c |   4 +
>  drivers/hid/hid-steam.c  | 480 
> +++
>  5 files changed, 497 insertions(+)
>  create mode 100644 drivers/hid/hid-steam.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 19c499f5623d..6e80fbf04e03 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -823,6 +823,14 @@ config HID_SPEEDLINK
> ---help---
> Support for Speedlink Vicious and Divine Cezanne mouse.
>
> +config HID_STEAM
> +   tristate "Steam Controller support"
> +   depends on HID
> +   ---help---
> +   Say Y here if you have a Steam Controller if you want to use it
> +   without running the Steam Client. It supports both the wired and
> +   the wireless adaptor.
> +
>  config HID_STEELSERIES
> tristate "Steelseries SRW-S1 steering wheel support"
> depends on HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index eb13b9e92d85..60a8abf84682 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -95,6 +95,7 @@ obj-$(CONFIG_HID_SAMSUNG) += hid-samsung.o
>  obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
>  obj-$(CONFIG_HID_SONY) += hid-sony.o
>  obj-$(CONFIG_HID_SPEEDLINK)+= hid-speedlink.o
> +obj-$(CONFIG_HID_STEAM)+= hid-steam.o
>  obj-$(CONFIG_HID_STEELSERIES)  += hid-steelseries.o
>  obj-$(CONFIG_HID_SUNPLUS)  += hid-sunplus.o
>  obj-$(CONFIG_HID_GREENASIA)+= hid-gaff.o
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 43ddcdfbd0da..be31a3c20818 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -988,6 +988,10 @@
>  #define USB_VENDOR_ID_STANTUM_SITRONIX 0x1403
>  #define USB_DEVICE_ID_MTP_SITRONIX 0x5001
>
> +#define USB_VENDOR_ID_VALVE0x28de
> +#define USB_DEVICE_ID_STEAM_CONTROLLER 0x1102
> +#define USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS0x1142
> +
>  #define USB_VENDOR_ID_STEELSERIES  0x1038
>  #define USB_DEVICE_ID_STEELSERIES_SRWS10x1410
>
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 5f6035a5ce36..72ac972dc00b 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -629,6 +629,10 @@ static const struct hid_device_id 
> hid_have_special_driver[] = {
>  #if IS_ENABLED(CONFIG_HID_SPEEDLINK)
> { HID_USB_DEVICE(USB_VENDOR_ID_X_TENSIONS, 
> USB_DEVICE_ID_SPEEDLINK_VAD_CEZANNE) },
>  #endif
> +#if IS_ENABLED(CONFIG_HID_STEAM)
> +   { HID_USB_DEVICE(USB_VENDOR_ID_VALVE, USB_DEVICE_ID_STEAM_CONTROLLER) 
> },
> +   { HID_USB_DEVICE(USB_VENDOR_ID_VALVE, 
> USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS) },
> +#endif
>  #if IS_ENABLED(CONFIG_HID_STEELSERIES)
> { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 
> USB_DEVICE_ID_STEELSERIES_SRWS1) },
>  #endif
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> new file mode 100644
> index ..03f912ab5484
> --- /dev/null
> +++ b/drivers/hid/hid-steam.c
> @@ -0,0 +1,480 @@
> +// SPDX-License-Identifier: GPL-2.0

Non standard header

> +/*
> + * HID driver for Valve Steam Controller
> + *
> + * Supports both the wired and wireless interfaces.
> + *
> + * Copyright (c) 2018 Rodrigo Rivas Costa

Re: [PATCH 1/3] HID: add driver for Valve Steam Controller

2018-02-14 Thread Benjamin Tissoires 
Hi Rodrigo,

On Tue, Feb 13, 2018 at 1:03 PM, Rodrigo Rivas Costa
 wrote:
> There are two ways to connect the Steam Controller: directly to the USB
> or with the USB wireless adapter.  Both methods are similar, but the
> wireless adapter can connect up to 4 devices at the same time.
>
> The wired device will appear as 3 interfaces: a virtual mouse, a virtual
> keyboard and a custom HID device.
>
> The wireless device will appear as 5 interfaces: a virtual keyboard and
> 4 custom HID devices, that will remain silent until a device is actually
> connected.
>
> The custom HID device has a report descriptor with all vendor specific
> usages, so the hid-generic is not very useful. In a PC/SteamBox Valve
> Steam Client provices a software translation by using direct USB access
> and a creates a uinput virtual gamepad.

I think I had a look at this a while ago, and didn't want to interfere
with SteamOS regarding this. I think your patch should be fine in that
regard, but have you tried SteamOS on a kernel patched with your
series? Does it behave properly or will it break?

See more comments inlined.

>
> This driver was reverse engineered to provide direct kernel support in
> case you cannot, or do not want to, use Valve Steam Client. It disables
> the virtual keyboard and mouse, as they are not so useful when you have
> a working gamepad.
>
> Working: buttons, axes, pads, wireless connect/disconnect.
>
> TO-DO: Battery, force-feedback, accelerometer/gyro, led, beeper...
>
> Signed-off-by: Rodrigo Rivas Costa 
> ---
>  drivers/hid/Kconfig  |   8 +
>  drivers/hid/Makefile |   1 +
>  drivers/hid/hid-ids.h|   4 +
>  drivers/hid/hid-quirks.c |   4 +
>  drivers/hid/hid-steam.c  | 480 
> +++
>  5 files changed, 497 insertions(+)
>  create mode 100644 drivers/hid/hid-steam.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 19c499f5623d..6e80fbf04e03 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -823,6 +823,14 @@ config HID_SPEEDLINK
> ---help---
> Support for Speedlink Vicious and Divine Cezanne mouse.
>
> +config HID_STEAM
> +   tristate "Steam Controller support"
> +   depends on HID
> +   ---help---
> +   Say Y here if you have a Steam Controller if you want to use it
> +   without running the Steam Client. It supports both the wired and
> +   the wireless adaptor.
> +
>  config HID_STEELSERIES
> tristate "Steelseries SRW-S1 steering wheel support"
> depends on HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index eb13b9e92d85..60a8abf84682 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -95,6 +95,7 @@ obj-$(CONFIG_HID_SAMSUNG) += hid-samsung.o
>  obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
>  obj-$(CONFIG_HID_SONY) += hid-sony.o
>  obj-$(CONFIG_HID_SPEEDLINK)+= hid-speedlink.o
> +obj-$(CONFIG_HID_STEAM)+= hid-steam.o
>  obj-$(CONFIG_HID_STEELSERIES)  += hid-steelseries.o
>  obj-$(CONFIG_HID_SUNPLUS)  += hid-sunplus.o
>  obj-$(CONFIG_HID_GREENASIA)+= hid-gaff.o
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 43ddcdfbd0da..be31a3c20818 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -988,6 +988,10 @@
>  #define USB_VENDOR_ID_STANTUM_SITRONIX 0x1403
>  #define USB_DEVICE_ID_MTP_SITRONIX 0x5001
>
> +#define USB_VENDOR_ID_VALVE0x28de
> +#define USB_DEVICE_ID_STEAM_CONTROLLER 0x1102
> +#define USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS0x1142
> +
>  #define USB_VENDOR_ID_STEELSERIES  0x1038
>  #define USB_DEVICE_ID_STEELSERIES_SRWS10x1410
>
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 5f6035a5ce36..72ac972dc00b 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -629,6 +629,10 @@ static const struct hid_device_id 
> hid_have_special_driver[] = {
>  #if IS_ENABLED(CONFIG_HID_SPEEDLINK)
> { HID_USB_DEVICE(USB_VENDOR_ID_X_TENSIONS, 
> USB_DEVICE_ID_SPEEDLINK_VAD_CEZANNE) },
>  #endif
> +#if IS_ENABLED(CONFIG_HID_STEAM)
> +   { HID_USB_DEVICE(USB_VENDOR_ID_VALVE, USB_DEVICE_ID_STEAM_CONTROLLER) 
> },
> +   { HID_USB_DEVICE(USB_VENDOR_ID_VALVE, 
> USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS) },
> +#endif
>  #if IS_ENABLED(CONFIG_HID_STEELSERIES)
> { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 
> USB_DEVICE_ID_STEELSERIES_SRWS1) },
>  #endif
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> new file mode 100644
> index ..03f912ab5484
> --- /dev/null
> +++ b/drivers/hid/hid-steam.c
> @@ -0,0 +1,480 @@
> +// SPDX-License-Identifier: GPL-2.0

Non standard header

> +/*
> + * HID driver for Valve Steam Controller
> + *
> + * Supports both the wired and wireless interfaces.
> + *
> + * Copyright (c) 2018 Rodrigo Rivas Costa 
> + */
> +
> +/*
> + * This program is free software; you can

[PATCH 1/3] HID: add driver for Valve Steam Controller

2018-02-13 Thread Rodrigo Rivas Costa 
There are two ways to connect the Steam Controller: directly to the USB
or with the USB wireless adapter.  Both methods are similar, but the
wireless adapter can connect up to 4 devices at the same time.

The wired device will appear as 3 interfaces: a virtual mouse, a virtual
keyboard and a custom HID device.

The wireless device will appear as 5 interfaces: a virtual keyboard and
4 custom HID devices, that will remain silent until a device is actually
connected.

The custom HID device has a report descriptor with all vendor specific
usages, so the hid-generic is not very useful. In a PC/SteamBox Valve
Steam Client provices a software translation by using direct USB access
and a creates a uinput virtual gamepad.

This driver was reverse engineered to provide direct kernel support in
case you cannot, or do not want to, use Valve Steam Client. It disables
the virtual keyboard and mouse, as they are not so useful when you have
a working gamepad.

Working: buttons, axes, pads, wireless connect/disconnect.

TO-DO: Battery, force-feedback, accelerometer/gyro, led, beeper...

Signed-off-by: Rodrigo Rivas Costa 
---
 drivers/hid/Kconfig  |   8 +
 drivers/hid/Makefile |   1 +
 drivers/hid/hid-ids.h|   4 +
 drivers/hid/hid-quirks.c |   4 +
 drivers/hid/hid-steam.c  | 480 +++
 5 files changed, 497 insertions(+)
 create mode 100644 drivers/hid/hid-steam.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 19c499f5623d..6e80fbf04e03 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -823,6 +823,14 @@ config HID_SPEEDLINK
---help---
Support for Speedlink Vicious and Divine Cezanne mouse.
 
+config HID_STEAM
+   tristate "Steam Controller support"
+   depends on HID
+   ---help---
+   Say Y here if you have a Steam Controller if you want to use it
+   without running the Steam Client. It supports both the wired and
+   the wireless adaptor.
+
 config HID_STEELSERIES
tristate "Steelseries SRW-S1 steering wheel support"
depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index eb13b9e92d85..60a8abf84682 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -95,6 +95,7 @@ obj-$(CONFIG_HID_SAMSUNG) += hid-samsung.o
 obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
 obj-$(CONFIG_HID_SONY) += hid-sony.o
 obj-$(CONFIG_HID_SPEEDLINK)+= hid-speedlink.o
+obj-$(CONFIG_HID_STEAM)+= hid-steam.o
 obj-$(CONFIG_HID_STEELSERIES)  += hid-steelseries.o
 obj-$(CONFIG_HID_SUNPLUS)  += hid-sunplus.o
 obj-$(CONFIG_HID_GREENASIA)+= hid-gaff.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 43ddcdfbd0da..be31a3c20818 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -988,6 +988,10 @@
 #define USB_VENDOR_ID_STANTUM_SITRONIX 0x1403
 #define USB_DEVICE_ID_MTP_SITRONIX 0x5001
 
+#define USB_VENDOR_ID_VALVE0x28de
+#define USB_DEVICE_ID_STEAM_CONTROLLER 0x1102
+#define USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS0x1142
+
 #define USB_VENDOR_ID_STEELSERIES  0x1038
 #define USB_DEVICE_ID_STEELSERIES_SRWS10x1410
 
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 5f6035a5ce36..72ac972dc00b 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -629,6 +629,10 @@ static const struct hid_device_id 
hid_have_special_driver[] = {
 #if IS_ENABLED(CONFIG_HID_SPEEDLINK)
{ HID_USB_DEVICE(USB_VENDOR_ID_X_TENSIONS, 
USB_DEVICE_ID_SPEEDLINK_VAD_CEZANNE) },
 #endif
+#if IS_ENABLED(CONFIG_HID_STEAM)
+   { HID_USB_DEVICE(USB_VENDOR_ID_VALVE, USB_DEVICE_ID_STEAM_CONTROLLER) },
+   { HID_USB_DEVICE(USB_VENDOR_ID_VALVE, 
USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS) },
+#endif
 #if IS_ENABLED(CONFIG_HID_STEELSERIES)
{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 
USB_DEVICE_ID_STEELSERIES_SRWS1) },
 #endif
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
new file mode 100644
index ..03f912ab5484
--- /dev/null
+++ b/drivers/hid/hid-steam.c
@@ -0,0 +1,480 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID driver for Valve Steam Controller
+ *
+ * Supports both the wired and wireless interfaces.
+ *
+ * Copyright (c) 2018 Rodrigo Rivas Costa 
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "hid-ids.h"
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Rodrigo Rivas Costa ");
+
+#define STEAM_QUIRK_WIRELESS   BIT(0)
+
+struct steam_device {
+   spinlock_t lock;
+   struct hid_device *hid_dev;
+   struct

[PATCH 1/3] HID: add driver for Valve Steam Controller

2018-02-13 Thread Rodrigo Rivas Costa 
There are two ways to connect the Steam Controller: directly to the USB
or with the USB wireless adapter.  Both methods are similar, but the
wireless adapter can connect up to 4 devices at the same time.

The wired device will appear as 3 interfaces: a virtual mouse, a virtual
keyboard and a custom HID device.

The wireless device will appear as 5 interfaces: a virtual keyboard and
4 custom HID devices, that will remain silent until a device is actually
connected.

The custom HID device has a report descriptor with all vendor specific
usages, so the hid-generic is not very useful. In a PC/SteamBox Valve
Steam Client provices a software translation by using direct USB access
and a creates a uinput virtual gamepad.

This driver was reverse engineered to provide direct kernel support in
case you cannot, or do not want to, use Valve Steam Client. It disables
the virtual keyboard and mouse, as they are not so useful when you have
a working gamepad.

Working: buttons, axes, pads, wireless connect/disconnect.

TO-DO: Battery, force-feedback, accelerometer/gyro, led, beeper...

Signed-off-by: Rodrigo Rivas Costa 
---
 drivers/hid/Kconfig  |   8 +
 drivers/hid/Makefile |   1 +
 drivers/hid/hid-ids.h|   4 +
 drivers/hid/hid-quirks.c |   4 +
 drivers/hid/hid-steam.c  | 480 +++
 5 files changed, 497 insertions(+)
 create mode 100644 drivers/hid/hid-steam.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 19c499f5623d..6e80fbf04e03 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -823,6 +823,14 @@ config HID_SPEEDLINK
---help---
Support for Speedlink Vicious and Divine Cezanne mouse.
 
+config HID_STEAM
+   tristate "Steam Controller support"
+   depends on HID
+   ---help---
+   Say Y here if you have a Steam Controller if you want to use it
+   without running the Steam Client. It supports both the wired and
+   the wireless adaptor.
+
 config HID_STEELSERIES
tristate "Steelseries SRW-S1 steering wheel support"
depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index eb13b9e92d85..60a8abf84682 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -95,6 +95,7 @@ obj-$(CONFIG_HID_SAMSUNG) += hid-samsung.o
 obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
 obj-$(CONFIG_HID_SONY) += hid-sony.o
 obj-$(CONFIG_HID_SPEEDLINK)+= hid-speedlink.o
+obj-$(CONFIG_HID_STEAM)+= hid-steam.o
 obj-$(CONFIG_HID_STEELSERIES)  += hid-steelseries.o
 obj-$(CONFIG_HID_SUNPLUS)  += hid-sunplus.o
 obj-$(CONFIG_HID_GREENASIA)+= hid-gaff.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 43ddcdfbd0da..be31a3c20818 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -988,6 +988,10 @@
 #define USB_VENDOR_ID_STANTUM_SITRONIX 0x1403
 #define USB_DEVICE_ID_MTP_SITRONIX 0x5001
 
+#define USB_VENDOR_ID_VALVE0x28de
+#define USB_DEVICE_ID_STEAM_CONTROLLER 0x1102
+#define USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS0x1142
+
 #define USB_VENDOR_ID_STEELSERIES  0x1038
 #define USB_DEVICE_ID_STEELSERIES_SRWS10x1410
 
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 5f6035a5ce36..72ac972dc00b 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -629,6 +629,10 @@ static const struct hid_device_id 
hid_have_special_driver[] = {
 #if IS_ENABLED(CONFIG_HID_SPEEDLINK)
{ HID_USB_DEVICE(USB_VENDOR_ID_X_TENSIONS, 
USB_DEVICE_ID_SPEEDLINK_VAD_CEZANNE) },
 #endif
+#if IS_ENABLED(CONFIG_HID_STEAM)
+   { HID_USB_DEVICE(USB_VENDOR_ID_VALVE, USB_DEVICE_ID_STEAM_CONTROLLER) },
+   { HID_USB_DEVICE(USB_VENDOR_ID_VALVE, 
USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS) },
+#endif
 #if IS_ENABLED(CONFIG_HID_STEELSERIES)
{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 
USB_DEVICE_ID_STEELSERIES_SRWS1) },
 #endif
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
new file mode 100644
index ..03f912ab5484
--- /dev/null
+++ b/drivers/hid/hid-steam.c
@@ -0,0 +1,480 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID driver for Valve Steam Controller
+ *
+ * Supports both the wired and wireless interfaces.
+ *
+ * Copyright (c) 2018 Rodrigo Rivas Costa 
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "hid-ids.h"
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Rodrigo Rivas Costa ");
+
+#define STEAM_QUIRK_WIRELESS   BIT(0)
+
+struct steam_device {
+   spinlock_t lock;
+   struct hid_device *hid_dev;
+   struct input_dev *input_dev;
+   unsigned long quirks;
+   struct work_struct work_connect;
+