On Fri, May 04, 2018 at 10:12:51AM +0200, Jiri Kosina wrote:

On Mon, 16 Apr 2018, Rodrigo Rivas Costa wrote:

quoted

Hello! This is reroll v9 of the Steam Controller driver.

@Pierre-Loup, @Clément, could you please check if this driver is acceptable by
Valve? I think this could be the one ;-).

I've rolled back the synthetic LPAD diagonals. It happens that the actual
coordinates of the pads are rotated about 15 degrees to the center, but the
marks in the pad are aligned to 0/90 degrees. That, and my poor's man
trigonometry, makes clicking the diagonals quite unreliable.
Sorry for the noise, but now I think that we are better limiting ourselves to
the events emitted by the hardware, as is.

Other than that, I've doing some torture testing, with and without the Steam
Client running. I've changed:
 1. The way EPIPE is handled when sending a report: I've noticed that Steam
    Client retries up to 50 times, with little or no delay between them, so I do
    the same.
 2. I've added a fallback in case this driver is unable to get the serial number
    of the controller. Failures on any other report can be ignored safely. But
    failing to get the serial number was preventing the creating of the hidraw
    node (wired controller only), and that could prevent Steam Client from
    working. 
 3. I've received a mail from Valve with a bunch of constants for the
    protocol. Most are related to the setting of new mappings (that we do not
    need) and a few of the rest are deprecated. Anyway, I've renamed the
    constants STEAM_CMD_* to align them with the official names, and added a
    few more that were unknown to me, just for future reference. Still, commands
    used for enabling/disabling lizard mode are the same.

If noone has any objections (last chance to raise them), I'll be queuing 
this for 4.18.

That would be great, thanks!

Now that my distro upgraded to 4.16, without the hid-quirks array, I've been
testing it a bit more. 

It works great, but I've noticed that I've missed a couple of lines when
creating the client hdev.  It doesn't seem to cause any harm, and anyway 'type'
and 'country' are currently both 0 in my controller; only 'version' is visibly
different but who uses that?

Anyway, this would be the change, I don't know if it is worth a whole reroll...

diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index af7ebb618867..cb86cc834201 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c

@@ -651,6 +651,9 @@ static struct hid_device *steam_create_client_hid(struct hid_device *hdev)
        client_hdev->bus = hdev->bus;
        client_hdev->vendor = hdev->vendor;
        client_hdev->product = hdev->product;
+       client_hdev->version = hdev->version;
+       client_hdev->type = hdev->type;
+       client_hdev->country = hdev->country;
        strlcpy(client_hdev->name, hdev->name,
                        sizeof(client_hdev->name));
        strlcpy(client_hdev->phys, hdev->phys,

Best regards,
--
Rodrigo Rivas Costa

Thanks,

-- 
Jiri Kosina
SUSE Labs