Re: [PATCH v5 0/4] new driver for Valve Steam Controller
On Wed, Mar 21, 2018 at 04:47:53PM +0100, Benjamin Tissoires wrote: > Hi Rodrigo, > > On Mon, Mar 19, 2018 at 9:08 PM, Rodrigo Rivas Costa > wrote: > > On Sat, Mar 17, 2018 at 02:54:07PM -0700, Pierre-Loup A. Griffais wrote: > >> > >> > >> On 03/15/2018 02:06 PM, Rodrigo Rivas Costa wrote: > >> > On Wed, Mar 14, 2018 at 05:39:25PM +0100, Benjamin Tissoires wrote: > >> > > On Mon, Mar 12, 2018 at 9:51 PM, Rodrigo Rivas Costa > >> > > wrote: > >> > > > On Mon, Mar 12, 2018 at 03:30:43PM +0100, Clément VUCHENER wrote: > >> > > > > 2018-03-11 20:58 GMT+01:00 Rodrigo Rivas Costa > >> > > > > : > >> > > > > > This patchset implements a driver for Valve Steam Controller, > >> > > > > > based on a > >> > > > > > reverse analysis by myself. > >> > > > > > > >> > > > > > Sorry, I've been out of town for a few weeks and couldn't keep > >> > > > > > up with this... > >> > > > > > > >> > > > > > @Pierre-Loup and @Clément, could you please have another look at > >> > > > > > this and > >> > > > > > check if it is worthy? Benjamin will not commit it without an > >> > > > > > express ACK from > >> > > > > > Valve. Of course he is right to be cautious, but I checked this > >> > > > > > driver with > >> > > > > > the Steam Client and all seems to go just fine. I think that > >> > > > > > there is a lot of > >> > > > > > Linux out of the desktop that could use this driver and cannot > >> > > > > > use the Steam > >> > > > > > Client. Worst case scenario, this driver can now be blacklisted, > >> > > > > > but I hope > >> > > > > > that will not be needed. > >> > > > > > >> > > > > I tested the driver with my 4.15 fedora kernel (I only built the > >> > > > > module not the whole kernel) and I got double inputs (your driver > >> > > > > input device + steam uinput device) when testing Shovel Knight with > >> > > > > Steam Big Picture. It seems to work fine when the inputs are the > >> > > > > same, > >> > > > > but after changing the controller configuration in Steam, the issue > >> > > > > became apparent. > >> > > > > >> > > > I assumed that when several joysticks are available, games would > >> > > > listen > >> > > > to one one of them. It looks like I'm wrong, and some (many?) games > >> > > > will > >> > > > listen to all available joysticks at the same time. Thus having two > >> > > > logical joysticks that represent the same physical one is not good. > >> > > > >> > > Yeah, the general rule of thumb is "think of the worst thing that can > >> > > happen, someone will do something worst". > >> > > > >> > > > > >> > > > An easy solution would be that Steam Client grabs this driver > >> > > > (ioctl(EVIOCGRAB)) when creating the uinput device. Another solution > >> > > > would be that Steam Client blacklists this driver, of course. > >> > > > >> > > This is 2 solutions that rely on a userspace change, and this is not > >> > > acceptable in its current form. What if people do not upgrade Steam > >> > > client but upgrade their kernel? Well, Steam might be able to force > >> > > people to always run the latest shiny available version, but for other > >> > > games, you can't expect people to have a compatible version of the > >> > > userspace stack. > >> > > >> > Well, if you don't have Steam then you don't have the double input in > >> > the first place. Unless you are using a different user-mode driver, of > >> > course. > >> > > > >> > > Also, "blacklisting the driver" from Steam client is something the OS > >> > > can do, but not the client when you run on a different distribution. > >> > > You need root for that, and I don't want to give root permissions to > >> > > Steam (or to any user space client that shouldn't have root privileges > >> > > for what it matters). > >> > > >> > Actually Steam needs a system installation that adds udev rules to grant > >> > uaccess to /dev/uinput and the USB raw device for the controller. > >> > Adding a /etc/modprobe.d/steam.conf should be possible, too. It would be > >> > a bit inconvenient because you'll need a distro update of the steam > >> > package, not just the usual user-mode-only auto-update. > >> > >> It's definitely a bit tricky; we've rolled out an update to our reference > >> package whenever we've added support for new devices (the final Steam > >> Controller, direct PS4 gamepad led/gyro access through HID, HTC Vive and > >> its > >> myriad of onboard devices, bootloaders of all these things for firmware > >> updates, etc). Whenever we have to do that, the rollout never is as smooth > >> as desired: many users aren't using our own package; even on the > >> distributions we support directly. Then downstream distributions adopt > >> these > >> udev changes with various delays (sometimes never), and port them to their > >> own mechanism of doing things, since everyone has their own idea of a > >> robust > >> security model. I wish local sessions always had proper access to HID > >> devices connected to the physical computer the user is sitting at, but I > >>
Re: [PATCH v5 0/4] new driver for Valve Steam Controller
Hi Rodrigo, On Mon, Mar 19, 2018 at 9:08 PM, Rodrigo Rivas Costa wrote: > On Sat, Mar 17, 2018 at 02:54:07PM -0700, Pierre-Loup A. Griffais wrote: >> >> >> On 03/15/2018 02:06 PM, Rodrigo Rivas Costa wrote: >> > On Wed, Mar 14, 2018 at 05:39:25PM +0100, Benjamin Tissoires wrote: >> > > On Mon, Mar 12, 2018 at 9:51 PM, Rodrigo Rivas Costa >> > > wrote: >> > > > On Mon, Mar 12, 2018 at 03:30:43PM +0100, Clément VUCHENER wrote: >> > > > > 2018-03-11 20:58 GMT+01:00 Rodrigo Rivas Costa >> > > > > : >> > > > > > This patchset implements a driver for Valve Steam Controller, >> > > > > > based on a >> > > > > > reverse analysis by myself. >> > > > > > >> > > > > > Sorry, I've been out of town for a few weeks and couldn't keep up >> > > > > > with this... >> > > > > > >> > > > > > @Pierre-Loup and @Clément, could you please have another look at >> > > > > > this and >> > > > > > check if it is worthy? Benjamin will not commit it without an >> > > > > > express ACK from >> > > > > > Valve. Of course he is right to be cautious, but I checked this >> > > > > > driver with >> > > > > > the Steam Client and all seems to go just fine. I think that there >> > > > > > is a lot of >> > > > > > Linux out of the desktop that could use this driver and cannot use >> > > > > > the Steam >> > > > > > Client. Worst case scenario, this driver can now be blacklisted, >> > > > > > but I hope >> > > > > > that will not be needed. >> > > > > >> > > > > I tested the driver with my 4.15 fedora kernel (I only built the >> > > > > module not the whole kernel) and I got double inputs (your driver >> > > > > input device + steam uinput device) when testing Shovel Knight with >> > > > > Steam Big Picture. It seems to work fine when the inputs are the >> > > > > same, >> > > > > but after changing the controller configuration in Steam, the issue >> > > > > became apparent. >> > > > >> > > > I assumed that when several joysticks are available, games would listen >> > > > to one one of them. It looks like I'm wrong, and some (many?) games >> > > > will >> > > > listen to all available joysticks at the same time. Thus having two >> > > > logical joysticks that represent the same physical one is not good. >> > > >> > > Yeah, the general rule of thumb is "think of the worst thing that can >> > > happen, someone will do something worst". >> > > >> > > > >> > > > An easy solution would be that Steam Client grabs this driver >> > > > (ioctl(EVIOCGRAB)) when creating the uinput device. Another solution >> > > > would be that Steam Client blacklists this driver, of course. >> > > >> > > This is 2 solutions that rely on a userspace change, and this is not >> > > acceptable in its current form. What if people do not upgrade Steam >> > > client but upgrade their kernel? Well, Steam might be able to force >> > > people to always run the latest shiny available version, but for other >> > > games, you can't expect people to have a compatible version of the >> > > userspace stack. >> > >> > Well, if you don't have Steam then you don't have the double input in >> > the first place. Unless you are using a different user-mode driver, of >> > course. >> > > >> > > Also, "blacklisting the driver" from Steam client is something the OS >> > > can do, but not the client when you run on a different distribution. >> > > You need root for that, and I don't want to give root permissions to >> > > Steam (or to any user space client that shouldn't have root privileges >> > > for what it matters). >> > >> > Actually Steam needs a system installation that adds udev rules to grant >> > uaccess to /dev/uinput and the USB raw device for the controller. >> > Adding a /etc/modprobe.d/steam.conf should be possible, too. It would be >> > a bit inconvenient because you'll need a distro update of the steam >> > package, not just the usual user-mode-only auto-update. >> >> It's definitely a bit tricky; we've rolled out an update to our reference >> package whenever we've added support for new devices (the final Steam >> Controller, direct PS4 gamepad led/gyro access through HID, HTC Vive and its >> myriad of onboard devices, bootloaders of all these things for firmware >> updates, etc). Whenever we have to do that, the rollout never is as smooth >> as desired: many users aren't using our own package; even on the >> distributions we support directly. Then downstream distributions adopt these >> udev changes with various delays (sometimes never), and port them to their >> own mechanism of doing things, since everyone has their own idea of a robust >> security model. I wish local sessions always had proper access to HID >> devices connected to the physical computer the user is sitting at, but I >> realize that the basic gaming desktop is just one of many usecases distros >> out there have to support and it'd be unreasonable to expect them to focus >> exclusively on it. > > I was afraid of something like that. Let's forget about that option for > the moment. > >> > > > > And
Re: [PATCH v5 0/4] new driver for Valve Steam Controller
On Mon, Mar 19, 2018 at 10:06:09PM +0100, Clément VUCHENER wrote:
> 2018-03-19 21:08 GMT+01:00 Rodrigo Rivas Costa :
> > On Sat, Mar 17, 2018 at 02:54:07PM -0700, Pierre-Loup A. Griffais wrote:
> >
> > Now, what I would really want is a review by Valve of my set-lizard
> > function:
> >
> > static void steam_set_lizard_mode(struct steam_device *steam, bool enabled)
> > {
> > if (enabled) {
> > steam_send_report_byte(steam, 0x8e); //enable mouse
> > steam_send_report_byte(steam, 0x85); //enable esc, enter
> > and cursor keys
> > } else {
> > steam_send_report_byte(steam, 0x81); //disable esc, enter
> > and cursor keys
> > steam_write_register(steam, 0x08, 0x07); //disable mouse
> > (cmd: 0x87)
> > }
> > }
> >
> > While it works, I find its asymmetry quite uncanny. I'm afraid that some
> > of these are there for a side effect, this is not their real purpose.
> > Could you give me a hint about this?
> >
>
> If I remember correctly, you can also enable the mouse with "87 03 08
> 00 00". But that do not explain the asymmetry or why there are two
> ways of doing it. I always found it weird that the "enable" value was
> 0x and the "disable" value 0x0007.
This works fine, thanks. IMO, it is better than command 0x8e.
I also found that register 0x07 controls the cursor keys emulation:
* 87 03 07 07 00: disable
* 87 03 07 03 00: enable (03 or 03
* 87 03 07 00 00: set "joystick mode" (?)
But I cannot find a similar register to disable the enter/esc keys, for
that I still need commands 0x85 and 0x81.
Can you tell me if there is a register to configure the enter/esc
emulation? That would be nice, because I could enable disable the lizard
mode with a single report: (87 09 07 07 00 08 07 00 xx 07 00).
Thanks.
Rodrigo
Re: [PATCH v5 0/4] new driver for Valve Steam Controller
2018-03-19 21:08 GMT+01:00 Rodrigo Rivas Costa :
> On Sat, Mar 17, 2018 at 02:54:07PM -0700, Pierre-Loup A. Griffais wrote:
>
> Now, what I would really want is a review by Valve of my set-lizard function:
>
> static void steam_set_lizard_mode(struct steam_device *steam, bool enabled)
> {
> if (enabled) {
> steam_send_report_byte(steam, 0x8e); //enable mouse
> steam_send_report_byte(steam, 0x85); //enable esc, enter and
> cursor keys
> } else {
> steam_send_report_byte(steam, 0x81); //disable esc, enter and
> cursor keys
> steam_write_register(steam, 0x08, 0x07); //disable mouse
> (cmd: 0x87)
> }
> }
>
> While it works, I find its asymmetry quite uncanny. I'm afraid that some
> of these are there for a side effect, this is not their real purpose.
> Could you give me a hint about this?
>
If I remember correctly, you can also enable the mouse with "87 03 08
00 00". But that do not explain the asymmetry or why there are two
ways of doing it. I always found it weird that the "enable" value was
0x and the "disable" value 0x0007.
Re: [PATCH v5 0/4] new driver for Valve Steam Controller
On Sat, Mar 17, 2018 at 02:54:07PM -0700, Pierre-Loup A. Griffais wrote: > > > On 03/15/2018 02:06 PM, Rodrigo Rivas Costa wrote: > > On Wed, Mar 14, 2018 at 05:39:25PM +0100, Benjamin Tissoires wrote: > > > On Mon, Mar 12, 2018 at 9:51 PM, Rodrigo Rivas Costa > > > wrote: > > > > On Mon, Mar 12, 2018 at 03:30:43PM +0100, Clément VUCHENER wrote: > > > > > 2018-03-11 20:58 GMT+01:00 Rodrigo Rivas Costa > > > > > : > > > > > > This patchset implements a driver for Valve Steam Controller, based > > > > > > on a > > > > > > reverse analysis by myself. > > > > > > > > > > > > Sorry, I've been out of town for a few weeks and couldn't keep up > > > > > > with this... > > > > > > > > > > > > @Pierre-Loup and @Clément, could you please have another look at > > > > > > this and > > > > > > check if it is worthy? Benjamin will not commit it without an > > > > > > express ACK from > > > > > > Valve. Of course he is right to be cautious, but I checked this > > > > > > driver with > > > > > > the Steam Client and all seems to go just fine. I think that there > > > > > > is a lot of > > > > > > Linux out of the desktop that could use this driver and cannot use > > > > > > the Steam > > > > > > Client. Worst case scenario, this driver can now be blacklisted, > > > > > > but I hope > > > > > > that will not be needed. > > > > > > > > > > I tested the driver with my 4.15 fedora kernel (I only built the > > > > > module not the whole kernel) and I got double inputs (your driver > > > > > input device + steam uinput device) when testing Shovel Knight with > > > > > Steam Big Picture. It seems to work fine when the inputs are the same, > > > > > but after changing the controller configuration in Steam, the issue > > > > > became apparent. > > > > > > > > I assumed that when several joysticks are available, games would listen > > > > to one one of them. It looks like I'm wrong, and some (many?) games will > > > > listen to all available joysticks at the same time. Thus having two > > > > logical joysticks that represent the same physical one is not good. > > > > > > Yeah, the general rule of thumb is "think of the worst thing that can > > > happen, someone will do something worst". > > > > > > > > > > > An easy solution would be that Steam Client grabs this driver > > > > (ioctl(EVIOCGRAB)) when creating the uinput device. Another solution > > > > would be that Steam Client blacklists this driver, of course. > > > > > > This is 2 solutions that rely on a userspace change, and this is not > > > acceptable in its current form. What if people do not upgrade Steam > > > client but upgrade their kernel? Well, Steam might be able to force > > > people to always run the latest shiny available version, but for other > > > games, you can't expect people to have a compatible version of the > > > userspace stack. > > > > Well, if you don't have Steam then you don't have the double input in > > the first place. Unless you are using a different user-mode driver, of > > course. > > > > > > Also, "blacklisting the driver" from Steam client is something the OS > > > can do, but not the client when you run on a different distribution. > > > You need root for that, and I don't want to give root permissions to > > > Steam (or to any user space client that shouldn't have root privileges > > > for what it matters). > > > > Actually Steam needs a system installation that adds udev rules to grant > > uaccess to /dev/uinput and the USB raw device for the controller. > > Adding a /etc/modprobe.d/steam.conf should be possible, too. It would be > > a bit inconvenient because you'll need a distro update of the steam > > package, not just the usual user-mode-only auto-update. > > It's definitely a bit tricky; we've rolled out an update to our reference > package whenever we've added support for new devices (the final Steam > Controller, direct PS4 gamepad led/gyro access through HID, HTC Vive and its > myriad of onboard devices, bootloaders of all these things for firmware > updates, etc). Whenever we have to do that, the rollout never is as smooth > as desired: many users aren't using our own package; even on the > distributions we support directly. Then downstream distributions adopt these > udev changes with various delays (sometimes never), and port them to their > own mechanism of doing things, since everyone has their own idea of a robust > security model. I wish local sessions always had proper access to HID > devices connected to the physical computer the user is sitting at, but I > realize that the basic gaming desktop is just one of many usecases distros > out there have to support and it'd be unreasonable to expect them to focus > exclusively on it. I was afraid of something like that. Let's forget about that option for the moment. > > > > > And without Steam and your external tool, you get double inputs too. I > > > > > tried RetroArch and it was unusable because of the keyboard inputs > > > > > from the lizard mode (e
Re: [PATCH v5 0/4] new driver for Valve Steam Controller
On 03/15/2018 02:06 PM, Rodrigo Rivas Costa wrote: On Wed, Mar 14, 2018 at 05:39:25PM +0100, Benjamin Tissoires wrote: On Mon, Mar 12, 2018 at 9:51 PM, Rodrigo Rivas Costa wrote: On Mon, Mar 12, 2018 at 03:30:43PM +0100, Clément VUCHENER wrote: 2018-03-11 20:58 GMT+01:00 Rodrigo Rivas Costa : This patchset implements a driver for Valve Steam Controller, based on a reverse analysis by myself. Sorry, I've been out of town for a few weeks and couldn't keep up with this... @Pierre-Loup and @Clément, could you please have another look at this and check if it is worthy? Benjamin will not commit it without an express ACK from Valve. Of course he is right to be cautious, but I checked this driver with the Steam Client and all seems to go just fine. I think that there is a lot of Linux out of the desktop that could use this driver and cannot use the Steam Client. Worst case scenario, this driver can now be blacklisted, but I hope that will not be needed. I tested the driver with my 4.15 fedora kernel (I only built the module not the whole kernel) and I got double inputs (your driver input device + steam uinput device) when testing Shovel Knight with Steam Big Picture. It seems to work fine when the inputs are the same, but after changing the controller configuration in Steam, the issue became apparent. I assumed that when several joysticks are available, games would listen to one one of them. It looks like I'm wrong, and some (many?) games will listen to all available joysticks at the same time. Thus having two logical joysticks that represent the same physical one is not good. Yeah, the general rule of thumb is "think of the worst thing that can happen, someone will do something worst". An easy solution would be that Steam Client grabs this driver (ioctl(EVIOCGRAB)) when creating the uinput device. Another solution would be that Steam Client blacklists this driver, of course. This is 2 solutions that rely on a userspace change, and this is not acceptable in its current form. What if people do not upgrade Steam client but upgrade their kernel? Well, Steam might be able to force people to always run the latest shiny available version, but for other games, you can't expect people to have a compatible version of the userspace stack. Well, if you don't have Steam then you don't have the double input in the first place. Unless you are using a different user-mode driver, of course. Also, "blacklisting the driver" from Steam client is something the OS can do, but not the client when you run on a different distribution. You need root for that, and I don't want to give root permissions to Steam (or to any user space client that shouldn't have root privileges for what it matters). Actually Steam needs a system installation that adds udev rules to grant uaccess to /dev/uinput and the USB raw device for the controller. Adding a /etc/modprobe.d/steam.conf should be possible, too. It would be a bit inconvenient because you'll need a distro update of the steam package, not just the usual user-mode-only auto-update. It's definitely a bit tricky; we've rolled out an update to our reference package whenever we've added support for new devices (the final Steam Controller, direct PS4 gamepad led/gyro access through HID, HTC Vive and its myriad of onboard devices, bootloaders of all these things for firmware updates, etc). Whenever we have to do that, the rollout never is as smooth as desired: many users aren't using our own package; even on the distributions we support directly. Then downstream distributions adopt these udev changes with various delays (sometimes never), and port them to their own mechanism of doing things, since everyone has their own idea of a robust security model. I wish local sessions always had proper access to HID devices connected to the physical computer the user is sitting at, but I realize that the basic gaming desktop is just one of many usecases distros out there have to support and it'd be unreasonable to expect them to focus exclusively on it. And without Steam and your external tool, you get double inputs too. I tried RetroArch and it was unusable because of the keyboard inputs from the lizard mode (e.g. pressing B also presses Esc and quits RetroArch). Having to download and compile an external tool to make the driver work properly may be too difficult for the user. Your goal was to provide an alternative to user space drivers but now you actually depend on (a very simple) one. Yes, I noticed that. TBH, this driver without Steam Client or the user-space tool is not very nice, precisely because you'll get constant Escape and Enter presses, and most games react to those. Frankly speaking, I'm not sure how to proceed. I can think of the following options: 1.Steam Client installation could add a file to blacklist hid-steam, just as it adds a few udev rules. But what about RetroArch? And what if you install Steam but want to play SDL games that c
Re: [PATCH v5 0/4] new driver for Valve Steam Controller
On Wed, Mar 14, 2018 at 05:39:25PM +0100, Benjamin Tissoires wrote: > On Mon, Mar 12, 2018 at 9:51 PM, Rodrigo Rivas Costa > wrote: > > On Mon, Mar 12, 2018 at 03:30:43PM +0100, Clément VUCHENER wrote: > >> 2018-03-11 20:58 GMT+01:00 Rodrigo Rivas Costa > >> : > >> > This patchset implements a driver for Valve Steam Controller, based on a > >> > reverse analysis by myself. > >> > > >> > Sorry, I've been out of town for a few weeks and couldn't keep up with > >> > this... > >> > > >> > @Pierre-Loup and @Clément, could you please have another look at this and > >> > check if it is worthy? Benjamin will not commit it without an express > >> > ACK from > >> > Valve. Of course he is right to be cautious, but I checked this driver > >> > with > >> > the Steam Client and all seems to go just fine. I think that there is a > >> > lot of > >> > Linux out of the desktop that could use this driver and cannot use the > >> > Steam > >> > Client. Worst case scenario, this driver can now be blacklisted, but I > >> > hope > >> > that will not be needed. > >> > >> I tested the driver with my 4.15 fedora kernel (I only built the > >> module not the whole kernel) and I got double inputs (your driver > >> input device + steam uinput device) when testing Shovel Knight with > >> Steam Big Picture. It seems to work fine when the inputs are the same, > >> but after changing the controller configuration in Steam, the issue > >> became apparent. > > > > I assumed that when several joysticks are available, games would listen > > to one one of them. It looks like I'm wrong, and some (many?) games will > > listen to all available joysticks at the same time. Thus having two > > logical joysticks that represent the same physical one is not good. > > Yeah, the general rule of thumb is "think of the worst thing that can > happen, someone will do something worst". > > > > > An easy solution would be that Steam Client grabs this driver > > (ioctl(EVIOCGRAB)) when creating the uinput device. Another solution > > would be that Steam Client blacklists this driver, of course. > > This is 2 solutions that rely on a userspace change, and this is not > acceptable in its current form. What if people do not upgrade Steam > client but upgrade their kernel? Well, Steam might be able to force > people to always run the latest shiny available version, but for other > games, you can't expect people to have a compatible version of the > userspace stack. Well, if you don't have Steam then you don't have the double input in the first place. Unless you are using a different user-mode driver, of course. > > Also, "blacklisting the driver" from Steam client is something the OS > can do, but not the client when you run on a different distribution. > You need root for that, and I don't want to give root permissions to > Steam (or to any user space client that shouldn't have root privileges > for what it matters). Actually Steam needs a system installation that adds udev rules to grant uaccess to /dev/uinput and the USB raw device for the controller. Adding a /etc/modprobe.d/steam.conf should be possible, too. It would be a bit inconvenient because you'll need a distro update of the steam package, not just the usual user-mode-only auto-update. > > > >> And without Steam and your external tool, you get double inputs too. I > >> tried RetroArch and it was unusable because of the keyboard inputs > >> from the lizard mode (e.g. pressing B also presses Esc and quits > >> RetroArch). Having to download and compile an external tool to make > >> the driver work properly may be too difficult for the user. Your goal > >> was to provide an alternative to user space drivers but now you > >> actually depend on (a very simple) one. > > > > Yes, I noticed that. TBH, this driver without Steam Client or the > > user-space tool is not very nice, precisely because you'll get constant > > Escape and Enter presses, and most games react to those. > > > > Frankly speaking, I'm not sure how to proceed. I can think of the > > following options: > > 1.Steam Client installation could add a file to blacklist > >hid-steam, just as it adds a few udev rules. > > But what about RetroArch? And what if you install Steam but want to > play SDL games that could benefit from your driver? That is an issue of solution 1. I actually have the module blacklisted in my PC, and run `sudo modprobe hid-steam` to use SDL. > > 2.The default CONFIG_HID_STEAM can be changed to "n". Maybe only > >on the architectures for which there is a Steam Client available. > >This way DIY projects will still be able to use it. > > But this will make the decision to include or not the driver in > distributions harder. And if no distribution uses it, you won't have > free tests, and you will be alone to maintain it. So that's not ideal > either Could we set the default to 'y' in non-PC systems. It would be enabled in my Raspbian, for example... better than nothing. > > > 3.This driver could b
Re: [PATCH v5 0/4] new driver for Valve Steam Controller
On Mon, Mar 12, 2018 at 9:51 PM, Rodrigo Rivas Costa wrote: > On Mon, Mar 12, 2018 at 03:30:43PM +0100, Clément VUCHENER wrote: >> 2018-03-11 20:58 GMT+01:00 Rodrigo Rivas Costa : >> > This patchset implements a driver for Valve Steam Controller, based on a >> > reverse analysis by myself. >> > >> > Sorry, I've been out of town for a few weeks and couldn't keep up with >> > this... >> > >> > @Pierre-Loup and @Clément, could you please have another look at this and >> > check if it is worthy? Benjamin will not commit it without an express ACK >> > from >> > Valve. Of course he is right to be cautious, but I checked this driver with >> > the Steam Client and all seems to go just fine. I think that there is a >> > lot of >> > Linux out of the desktop that could use this driver and cannot use the >> > Steam >> > Client. Worst case scenario, this driver can now be blacklisted, but I hope >> > that will not be needed. >> >> I tested the driver with my 4.15 fedora kernel (I only built the >> module not the whole kernel) and I got double inputs (your driver >> input device + steam uinput device) when testing Shovel Knight with >> Steam Big Picture. It seems to work fine when the inputs are the same, >> but after changing the controller configuration in Steam, the issue >> became apparent. > > I assumed that when several joysticks are available, games would listen > to one one of them. It looks like I'm wrong, and some (many?) games will > listen to all available joysticks at the same time. Thus having two > logical joysticks that represent the same physical one is not good. Yeah, the general rule of thumb is "think of the worst thing that can happen, someone will do something worst". > > An easy solution would be that Steam Client grabs this driver > (ioctl(EVIOCGRAB)) when creating the uinput device. Another solution > would be that Steam Client blacklists this driver, of course. This is 2 solutions that rely on a userspace change, and this is not acceptable in its current form. What if people do not upgrade Steam client but upgrade their kernel? Well, Steam might be able to force people to always run the latest shiny available version, but for other games, you can't expect people to have a compatible version of the userspace stack. Also, "blacklisting the driver" from Steam client is something the OS can do, but not the client when you run on a different distribution. You need root for that, and I don't want to give root permissions to Steam (or to any user space client that shouldn't have root privileges for what it matters). > >> And without Steam and your external tool, you get double inputs too. I >> tried RetroArch and it was unusable because of the keyboard inputs >> from the lizard mode (e.g. pressing B also presses Esc and quits >> RetroArch). Having to download and compile an external tool to make >> the driver work properly may be too difficult for the user. Your goal >> was to provide an alternative to user space drivers but now you >> actually depend on (a very simple) one. > > Yes, I noticed that. TBH, this driver without Steam Client or the > user-space tool is not very nice, precisely because you'll get constant > Escape and Enter presses, and most games react to those. > > Frankly speaking, I'm not sure how to proceed. I can think of the > following options: > 1.Steam Client installation could add a file to blacklist >hid-steam, just as it adds a few udev rules. But what about RetroArch? And what if you install Steam but want to play SDL games that could benefit from your driver? > 2.The default CONFIG_HID_STEAM can be changed to "n". Maybe only >on the architectures for which there is a Steam Client available. >This way DIY projects will still be able to use it. But this will make the decision to include or not the driver in distributions harder. And if no distribution uses it, you won't have free tests, and you will be alone to maintain it. So that's not ideal either > 3.This driver could be abandoned :-(. Just use Steam Client if possible or >any of the user-mode drivers available. This would be a waste for everybody as it's always better when we share. > > If we decide for 1 or 2, then the lizard mode could be disabled without > ill effects. We could even enable the gyro and all the other gadgets > without worring about current compatibility. To me, 1 is out of the question. The kernel can't expect a user space change especially if you are knowingly introducing a bug for the end user. 2 is still on the table IMO, and 3 would be a shame. I know we already discussed about sysfs and module parameters, but if the driver will conflict with a userspace stack, the only way would be to have a (dynamic) parameter "enhanced_mode" or "kernel_awesome_support" or whatever which would be a boolean, that defaults to false that Steam can eventually lookup if they want so in the future we can default it to true. When this parameter is set, the driver will create the input
Re: [PATCH v5 0/4] new driver for Valve Steam Controller
On Mon, Mar 12, 2018 at 03:30:43PM +0100, Clément VUCHENER wrote: > 2018-03-11 20:58 GMT+01:00 Rodrigo Rivas Costa : > > This patchset implements a driver for Valve Steam Controller, based on a > > reverse analysis by myself. > > > > Sorry, I've been out of town for a few weeks and couldn't keep up with > > this... > > > > @Pierre-Loup and @Clément, could you please have another look at this and > > check if it is worthy? Benjamin will not commit it without an express ACK > > from > > Valve. Of course he is right to be cautious, but I checked this driver with > > the Steam Client and all seems to go just fine. I think that there is a lot > > of > > Linux out of the desktop that could use this driver and cannot use the Steam > > Client. Worst case scenario, this driver can now be blacklisted, but I hope > > that will not be needed. > > I tested the driver with my 4.15 fedora kernel (I only built the > module not the whole kernel) and I got double inputs (your driver > input device + steam uinput device) when testing Shovel Knight with > Steam Big Picture. It seems to work fine when the inputs are the same, > but after changing the controller configuration in Steam, the issue > became apparent. I assumed that when several joysticks are available, games would listen to one one of them. It looks like I'm wrong, and some (many?) games will listen to all available joysticks at the same time. Thus having two logical joysticks that represent the same physical one is not good. An easy solution would be that Steam Client grabs this driver (ioctl(EVIOCGRAB)) when creating the uinput device. Another solution would be that Steam Client blacklists this driver, of course. > And without Steam and your external tool, you get double inputs too. I > tried RetroArch and it was unusable because of the keyboard inputs > from the lizard mode (e.g. pressing B also presses Esc and quits > RetroArch). Having to download and compile an external tool to make > the driver work properly may be too difficult for the user. Your goal > was to provide an alternative to user space drivers but now you > actually depend on (a very simple) one. Yes, I noticed that. TBH, this driver without Steam Client or the user-space tool is not very nice, precisely because you'll get constant Escape and Enter presses, and most games react to those. Frankly speaking, I'm not sure how to proceed. I can think of the following options: 1.Steam Client installation could add a file to blacklist hid-steam, just as it adds a few udev rules. 2.The default CONFIG_HID_STEAM can be changed to "n". Maybe only on the architectures for which there is a Steam Client available. This way DIY projects will still be able to use it. 3.This driver could be abandoned :-(. Just use Steam Client if possible or any of the user-mode drivers available. If we decide for 1 or 2, then the lizard mode could be disabled without ill effects. We could even enable the gyro and all the other gadgets without worring about current compatibility. At the end of the day, I think that it is up to Valve what to do. Best Regards. Rodrigo. > Also the button and axis codes do not match the gamepad API doc > (https://www.kernel.org/doc/Documentation/input/gamepad.txt). > > > > > For full reference, I'm adding a full changelog of this patchset. > > > > Changes in v5: > > * Fix license SPDX to GPL-2.0+. > > * Minor stylistic changes (BIT(3) instead 0x08 and so on). > > > > Changes in v4: > > * Add command to check the wireless connection status on probe, without > >waiting for a message (thanks to Clément Vuchener for the tip). > > * Removed the error code on redundant connection/disconnection messages. > > That > >was harmless but polluted dmesg. > > * Added buttons for touching the left-pad and right-pad. > > * Fixed a misplaced #include from 2/4 to 1/4. > > > > Changes in v3: > > * Use RCU to do the dynamic connec/disconnect of wireless devices. > > * 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. > > * Add fuzz values for left/right pad axes, they are a little wiggly. > > > > Changes in v2: > > * 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 (4): > > HID: add driver for Valve Steam Controller > > HID: steam: add serial number information. > > HID: steam: command to check wireless c
Re: [PATCH v5 0/4] new driver for Valve Steam Controller
2018-03-11 20:58 GMT+01:00 Rodrigo Rivas Costa : > This patchset implements a driver for Valve Steam Controller, based on a > reverse analysis by myself. > > Sorry, I've been out of town for a few weeks and couldn't keep up with this... > > @Pierre-Loup and @Clément, could you please have another look at this and > check if it is worthy? Benjamin will not commit it without an express ACK from > Valve. Of course he is right to be cautious, but I checked this driver with > the Steam Client and all seems to go just fine. I think that there is a lot of > Linux out of the desktop that could use this driver and cannot use the Steam > Client. Worst case scenario, this driver can now be blacklisted, but I hope > that will not be needed. I tested the driver with my 4.15 fedora kernel (I only built the module not the whole kernel) and I got double inputs (your driver input device + steam uinput device) when testing Shovel Knight with Steam Big Picture. It seems to work fine when the inputs are the same, but after changing the controller configuration in Steam, the issue became apparent. And without Steam and your external tool, you get double inputs too. I tried RetroArch and it was unusable because of the keyboard inputs from the lizard mode (e.g. pressing B also presses Esc and quits RetroArch). Having to download and compile an external tool to make the driver work properly may be too difficult for the user. Your goal was to provide an alternative to user space drivers but now you actually depend on (a very simple) one. Also the button and axis codes do not match the gamepad API doc (https://www.kernel.org/doc/Documentation/input/gamepad.txt). > > For full reference, I'm adding a full changelog of this patchset. > > Changes in v5: > * Fix license SPDX to GPL-2.0+. > * Minor stylistic changes (BIT(3) instead 0x08 and so on). > > Changes in v4: > * Add command to check the wireless connection status on probe, without >waiting for a message (thanks to Clément Vuchener for the tip). > * Removed the error code on redundant connection/disconnection messages. That >was harmless but polluted dmesg. > * Added buttons for touching the left-pad and right-pad. > * Fixed a misplaced #include from 2/4 to 1/4. > > Changes in v3: > * Use RCU to do the dynamic connec/disconnect of wireless devices. > * 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. > * Add fuzz values for left/right pad axes, they are a little wiggly. > > Changes in v2: > * 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 (4): > HID: add driver for Valve Steam Controller > HID: steam: add serial number information. > HID: steam: command to check wireless connection > HID: steam: add battery device. > > drivers/hid/Kconfig | 8 + > drivers/hid/Makefile| 1 + > drivers/hid/hid-ids.h | 4 + > drivers/hid/hid-steam.c | 794 > > 4 files changed, 807 insertions(+) > create mode 100644 drivers/hid/hid-steam.c > > -- > 2.16.2 >
Re: [PATCH v5 0/4] new driver for Valve Steam Controller
On Sun, Mar 11, 2018 at 04:12:41PM -0700, Pierre-Loup A. Griffais wrote: > > > On 03/11/2018 12:58 PM, Rodrigo Rivas Costa wrote: > > This patchset implements a driver for Valve Steam Controller, based on a > > reverse analysis by myself. > > > > Sorry, I've been out of town for a few weeks and couldn't keep up with > > this... > > > > @Pierre-Loup and @Clément, could you please have another look at this and > > check if it is worthy? Benjamin will not commit it without an express ACK > > from > > Valve. Of course he is right to be cautious, but I checked this driver with > > the Steam Client and all seems to go just fine. I think that there is a lot > > of > > Linux out of the desktop that could use this driver and cannot use the Steam > > Client. Worst case scenario, this driver can now be blacklisted, but I hope > > that will not be needed. > > Hi Rodrigo, > > I think the approach you outlined earlier of only sending configuration > commands to the device when something is actively using the driver is sane. > I won't have the cycles to thoroughly check all the possible interactions > with the client in the near future, so in the interest of not blocking > development I'd say go for it. I'll try to get someone to take the patchset > for a spin soon. First of all, thank you very much for your attention. Note that this driver does not send any configuration command at all. It only sends commands get_serial (ae 15 01) and get_comm_status (b4), and those just on probe time. That should be quite safe IMO. Sending a configuration command, such as enable/disable_gyro on input->open() is probably not so safe, so I'm leaving that for an eventual future patch. Many games (and even X) enumerate the input devices by opening and closing them. That could cause trouble... Best regards. Rodrigo > > Thanks, > - Pierre-Loup > > > > > For full reference, I'm adding a full changelog of this patchset. > > > > Changes in v5: > > * Fix license SPDX to GPL-2.0+. > > * Minor stylistic changes (BIT(3) instead 0x08 and so on). > > > > Changes in v4: > > * Add command to check the wireless connection status on probe, without > > waiting for a message (thanks to Clément Vuchener for the tip). > > * Removed the error code on redundant connection/disconnection messages. > > That > > was harmless but polluted dmesg. > > * Added buttons for touching the left-pad and right-pad. > > * Fixed a misplaced #include from 2/4 to 1/4. > > > > Changes in v3: > > * Use RCU to do the dynamic connec/disconnect of wireless devices. > > * 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. > > * Add fuzz values for left/right pad axes, they are a little wiggly. > > > > Changes in v2: > > * 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 (4): > >HID: add driver for Valve Steam Controller > >HID: steam: add serial number information. > >HID: steam: command to check wireless connection > >HID: steam: add battery device. > > > > drivers/hid/Kconfig | 8 + > > drivers/hid/Makefile| 1 + > > drivers/hid/hid-ids.h | 4 + > > drivers/hid/hid-steam.c | 794 > > > > 4 files changed, 807 insertions(+) > > create mode 100644 drivers/hid/hid-steam.c > > >
Re: [PATCH v5 0/4] new driver for Valve Steam Controller
On 03/11/2018 12:58 PM, Rodrigo Rivas Costa wrote: This patchset implements a driver for Valve Steam Controller, based on a reverse analysis by myself. Sorry, I've been out of town for a few weeks and couldn't keep up with this... @Pierre-Loup and @Clément, could you please have another look at this and check if it is worthy? Benjamin will not commit it without an express ACK from Valve. Of course he is right to be cautious, but I checked this driver with the Steam Client and all seems to go just fine. I think that there is a lot of Linux out of the desktop that could use this driver and cannot use the Steam Client. Worst case scenario, this driver can now be blacklisted, but I hope that will not be needed. Hi Rodrigo, I think the approach you outlined earlier of only sending configuration commands to the device when something is actively using the driver is sane. I won't have the cycles to thoroughly check all the possible interactions with the client in the near future, so in the interest of not blocking development I'd say go for it. I'll try to get someone to take the patchset for a spin soon. Thanks, - Pierre-Loup For full reference, I'm adding a full changelog of this patchset. Changes in v5: * Fix license SPDX to GPL-2.0+. * Minor stylistic changes (BIT(3) instead 0x08 and so on). Changes in v4: * Add command to check the wireless connection status on probe, without waiting for a message (thanks to Clément Vuchener for the tip). * Removed the error code on redundant connection/disconnection messages. That was harmless but polluted dmesg. * Added buttons for touching the left-pad and right-pad. * Fixed a misplaced #include from 2/4 to 1/4. Changes in v3: * Use RCU to do the dynamic connec/disconnect of wireless devices. * 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. * Add fuzz values for left/right pad axes, they are a little wiggly. Changes in v2: * 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 (4): HID: add driver for Valve Steam Controller HID: steam: add serial number information. HID: steam: command to check wireless connection HID: steam: add battery device. drivers/hid/Kconfig | 8 + drivers/hid/Makefile| 1 + drivers/hid/hid-ids.h | 4 + drivers/hid/hid-steam.c | 794 4 files changed, 807 insertions(+) create mode 100644 drivers/hid/hid-steam.c
