Received: by 10.223.185.116 with SMTP id b49csp1021500wrg; Fri, 16 Feb 2018 10:59:03 -0800 (PST) X-Google-Smtp-Source: AH8x2263hopZZoXprl3IUoDy0MAejr6RzQfswfv9H7r/8kGh1k44Ajl3bZhwyHxHpH9i6Ws7GRph X-Received: by 2002:a17:902:be09:: with SMTP id r9-v6mr6780727pls.234.1518807543124; Fri, 16 Feb 2018 10:59:03 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1518807543; cv=none; d=google.com; s=arc-20160816; b=WaSTdvA0x/ixC/pGwWrLEG9dK7FW/spnjWOXmzEvuSruunXQaXigbOQZYfh2bFOcjG ZqHi3j8LngAH9aYFFoRoajAsQ9q5fjW1JE6w+fEu+/yIwk411CtEfMB5ewEZ2PAg7eWS iYEfvNwCdotD+4mOybTVc4nJ8bK6hqy57gp9eh3xSMKdTJ96Rj2199bdRexB/2gRNmCt dlKJnQ/OQinO+Ie2bTsAqci7oNtdBkcqL4evOaw403dxZFpYaNBeFTDyE/ZFmPsR4+DQ f99kI/eBXHZ2s7NkUoeobmAbJS/mhl9smfrtpj6iXLKh5yf8XJImuxqFpaUrY61unyNM 1aCg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :references:in-reply-to:mime-version:arc-authentication-results; bh=aJeU/myWAaRLxd0vlj6ck+LJo/NT6rb6QX4Fxv8j0LA=; b=GTrT19GuopLNzaX62GG4tXlim7GIed160D/jXqffVN+B8yPxo7uS54O7hhnaq3ZrAR zPxmPOQRXOvN6CU1Iet37UR+9SQxoQ/Sr2d3Tf96cG+1AbS6P26FBUu0rOtQBq2MYe/q KSEzNU8l1Ucj6ayyhuwXSZWrxctb6UMU/AHygTffgEnWfVefWf81q7PHk52liOTyCUkT Gu/rAOvOTkTHKGC3j1z4sCEFPxk9bITB9d0CN9nqOZOg91rvc7lKd6/QCaqgpQZVIPoY l+c4DNuap24RMoTJb0IrbpNRJYcwAh49QhyfHJ/8V8GOZZMkUr5PrU8IjqjRaiAXgl8w Bxmw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id n4si2507749pgd.570.2018.02.16.10.58.48; Fri, 16 Feb 2018 10:59:03 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933624AbeBPKiN (ORCPT + 99 others); Fri, 16 Feb 2018 05:38:13 -0500 Received: from mail-qk0-f175.google.com ([209.85.220.175]:39064 "EHLO mail-qk0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933559AbeBPKiM (ORCPT ); Fri, 16 Feb 2018 05:38:12 -0500 Received: by mail-qk0-f175.google.com with SMTP id z197so3172610qkb.6 for ; Fri, 16 Feb 2018 02:38:12 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=aJeU/myWAaRLxd0vlj6ck+LJo/NT6rb6QX4Fxv8j0LA=; b=IvGHEGIm2PmY29mZecadUSPf7fK+XeLQWRpoEoF1dI1kQJReE/9+nc6OK3d+LJ+2f6 d32+XdU2HCATw+DO644UwI1TlZ75JyjWHIyQaohk7WC5NaOz9fEsc3hP3ZoyQ+EXI6/Q 1cJSYN/3hX4mNaH/FsVM1h2VoLTZMTiII1z9e4HPToc646kxpo1Lq972tyd+XzXF+d+U seleRN6uNncQe83rF1Aj0lT4DqJvE13o88xrQO0AhbJN8UZ89kkpt5zsmAWQEJ6G1Gk8 8MfUjj2Qs6HEOyvhK8NZA6JWO+c/tEDq1GAn4iKonlnVo/Fdp4Lb8WfdpSpP7ad8knas XdnA== X-Gm-Message-State: APf1xPD7z/Wj1Ef934rzPzHzC5wr0OtJfD6V9ZUagobyYM2E6zqrKzDO clyMaGaKLgQKzO/CTgSEYB3A+RObQMRRXf2fFFzM8w== X-Received: by 10.233.220.199 with SMTP id q190mr7969557qkf.285.1518777491699; Fri, 16 Feb 2018 02:38:11 -0800 (PST) MIME-Version: 1.0 Received: by 10.200.26.166 with HTTP; Fri, 16 Feb 2018 02:38:11 -0800 (PST) In-Reply-To: <20180216095722.GC18755@casa> References: <20180213120308.23879-1-rodrigorivascosta@gmail.com> <20180213120308.23879-2-rodrigorivascosta@gmail.com> <20180215221633.GA18755@casa> <20180216090243.GB18755@casa> <20180216095722.GC18755@casa> From: Benjamin Tissoires Date: Fri, 16 Feb 2018 11:38:11 +0100 Message-ID: Subject: Re: [PATCH 2/3] HID: steam: add serial number information. To: Rodrigo Rivas Costa Cc: Jiri Kosina , lkml , linux-input@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 16, 2018 at 10:57 AM, Rodrigo Rivas Costa wrote: > On Fri, Feb 16, 2018 at 10:31:35AM +0100, Benjamin Tissoires wrote: >> > Ok, I'll do that. The weird thing, however, is that: >> > >> > hid_hw_raw_request(steam->hid_dev, 0x00, >> > buf, hid_report_len(r), /* 64 */ >> > HID_FEATURE_REPORT, HID_REQ_GET_REPORT); >> > >> > fails with EOVERFLOW. I have to use: >> > >> > hid_hw_raw_request(steam->hid_dev, 0x00, >> > buf, 65 >> > HID_FEATURE_REPORT, HID_REQ_GET_REPORT); >> > >> > which just feels wrong to me. >> >> Indeed >> >> > >> > And looking around drivers/hid/*.c I see that most calls to >> > hid_hw_raw_request(..., HID_REQ_GET_REPORT) use a buffer allocated with >> > {devm_,}kzalloc() and a constant length, never using >> > hid_alloc_report_buf() or hid_report_len(). >> >> well, hid-input.c and hid-multitouch.c are using >> hid_alloc_report_buf() and these two are the most generic ones. We >> haven't converted everybody to use hid_alloc_report_buf(), but it's >> not a reason to not use it for new drivers :) > > Agreed. And they use hid_report_len() on the hid_hw_raw_request(). > > Now my guess is that hid_report_len() should return 65 instead of 64. > And it would do that if the report.id would be >0, but, alas, it is =0. > Maybe feature reports should add 1 without checking id>0? Or maybe > the Steam report descriptor is wrong and I should use report_fixup or > something? > >> No, it's unlikely that there is a bug. Can you forward to me the >> report descriptors of the Steam controller (ideally with the tool >> hid-recorder from http://bentiss.github.io/hid-replay-docs/, so I can >> get a few events too)? > > Sure, see the attached file. It is the wired one, I opened jstest and > moved the joystick around to get a few events. Unfortunately the > HID_REQ_GET_REPORT are not recorded. > > For the casual reader, here is the decoded report descriptor [1]: > > 0x06, 0x00, 0xFF, // Usage Page (Vendor Defined 0xFF00) > 0x09, 0x01, // Usage (0x01) > 0xA1, 0x01, // Collection (Application) > 0x15, 0x00, // Logical Minimum (0) > 0x26, 0xFF, 0x00, // Logical Maximum (255) > 0x75, 0x08, // Report Size (8) > 0x95, 0x40, // Report Count (64) > 0x09, 0x01, // Usage (0x01) > 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) > 0x95, 0x40, // Report Count (64) > 0x09, 0x01, // Usage (0x01) > 0x91, 0x02, // Output (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) > 0x95, 0x40, // Report Count (64) > 0x09, 0x01, // Usage (0x01) > 0xB1, 0x02, // Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) > 0xC0, // End Collection > > > [1]: http://eleccelerator.com/usbdescreqparser/ Thanks. Arf, looks like usbhid expects the buffer to start with 0x00 when the report is not numbered, thus adding one to the report length. I guess that nobody tried to recently set/get reports on a device without a report ID. And hidraw matches this behavior too, which means it's hard to change. One thing I'd like to try to change is the result of hid_report_len. If everybody expects the size to be of one more when the report is unnumbered, maybe we could simply add this placeholder in the report size from the beginning. We'd need more testing though that just my gut feeling that it's the right thing to do. Cheers, Benjamin