Received: by 10.223.185.116 with SMTP id b49csp848586wrg; Tue, 20 Feb 2018 08:49:55 -0800 (PST) X-Google-Smtp-Source: AH8x226VbHYJD486i4W4vbuyz37DeluDY8zxV4D6OuM6glskT+DXDycLjwy3+2BLI4gT7FI4lTXg X-Received: by 2002:a17:902:bd04:: with SMTP id p4-v6mr202491pls.253.1519145395197; Tue, 20 Feb 2018 08:49:55 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1519145395; cv=none; d=google.com; s=arc-20160816; b=cdJzRcWCsMPu80RByWEz4LD4aJquNNyyvK2+rQPlKxZbV/1YfOzZwU1Xh5CP7xjZaj Aq6jU+Em1fJ57eB+u6oXWPsIsRNFJiliA+diVORL4Gx2AdpQfsVKkDqmnXiniU46Zk7m xa4IyMpW48mJvKhSbNaz0gZ7SIbP4bMX7R/g1Lz/+n7t1ag1hBCjlVTpTvcJtsdVkp0z +M7kac9W+IOur+qlOlQgW4tfJ/LuyikPamKTbzmN/3oFrRjHjrHOqIsOYLg929lV+oOV eQUnGYnaf+sciUy/0Co+dy6wJQyTOtoQDJvVtEwzOs1ahgVfJbD/T3uQrAqT6ImGN2Lp AEig== 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=je7DGNIlqn0Akq/5o1u9SA245RHp2nL61szhSK+yBpc=; b=AXS+fjYRRts4ncptba0IJThcFtFyYUbV3rtRfDnqOrMpqdDmi6eczGEmnEWkcnoG/R /OSnwpWgl63mXzW9/FTbGwCxdzW76MaYVP/Iz1NPBkxHhAiWFo7Jjqeogi0pq2CiGHyy BfNssJyzber3zALly+3Gh8FGReJmFgnqxyag263LkVV1FOyPMZEQhBbR7SbtnL1j2cLM DHEalZNyJpoYbtQ9Zve1cNad2pyNSXciCs0dCzU+V2HTjCQWbtWbUTuzPKdQVhNuYbso HFsLEjUmtQ7gVLzjM57d9igw0cE9HppyW2eTWOx35JhCms5hOYT/uUNdGFJcXocl0hrI hZ3Q== 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 i3-v6si826573plt.369.2018.02.20.08.49.40; Tue, 20 Feb 2018 08:49:55 -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 S1753159AbeBTQtF (ORCPT + 99 others); Tue, 20 Feb 2018 11:49:05 -0500 Received: from mail-qt0-f179.google.com ([209.85.216.179]:39012 "EHLO mail-qt0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753078AbeBTQtD (ORCPT ); Tue, 20 Feb 2018 11:49:03 -0500 Received: by mail-qt0-f179.google.com with SMTP id f4so17172195qtj.6 for ; Tue, 20 Feb 2018 08:49:03 -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=je7DGNIlqn0Akq/5o1u9SA245RHp2nL61szhSK+yBpc=; b=tTsolledVLEdxxj1+y1tO/yFThHGSOUZ+BXqaFw2+21hAfoz/ns+a9J51mfyDAVuQ2 BMz+0lPtBXVPyPYpmKEii/2hk4haHJkWjoJWiWqgVlVFGCoPHknXQn3MBCm6UxEjQ16s Ava9uCRiJnEzN2wsF/eW/hwwMXqADnw4+w9lQrd8r1wn1T1OwnJgj6d2W8Q24GWYVfXc O+ciuav2mcj/xl8VWGTM8hU4JOf5VJ0w/pR2XUBnMgGazuCM2zAPVSTS+O6YbMGUO9LC AQ+6lNj3vTXNYNguUwW9XbJSFMCwpiT6ed7eDvoExfSJqUUqwfWNJ+B1zyC4yTnc9PEc ExNw== X-Gm-Message-State: APf1xPBp3y/SBFULmDyOMDBukQWrmAG8ZGVTgKoC4odYhMamvM8vhYi1 503mr7R231ZIOCwkUwD+gpa0VMxcALnZHvvzBAPoFg== X-Received: by 10.200.53.137 with SMTP id k9mr390238qtb.37.1519145342637; Tue, 20 Feb 2018 08:49:02 -0800 (PST) MIME-Version: 1.0 Received: by 10.200.26.166 with HTTP; Tue, 20 Feb 2018 08:49:02 -0800 (PST) In-Reply-To: <20180216205939.GA17329@casa> References: <20180213120308.23879-1-rodrigorivascosta@gmail.com> <20180213120308.23879-2-rodrigorivascosta@gmail.com> <20180215221633.GA18755@casa> <20180216090243.GB18755@casa> <20180216095722.GC18755@casa> <20180216205939.GA17329@casa> From: Benjamin Tissoires Date: Tue, 20 Feb 2018 17:49:02 +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 9:59 PM, Rodrigo Rivas Costa wrote: > On Fri, Feb 16, 2018 at 11:38:11AM +0100, Benjamin Tissoires wrote: >> 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 >> >> >> >> 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. > > I've been trying to make sense of all this numbered/buf++/count-- stuff, > and I admit I don't understand half of it... > > But about that +7 in hid_alloc_report_buf(), isn't it to make room for > the implement()/extract() functions? And IIUIC those are not used for > raw_requests... they are instead passed directly to usb_control_msg() > (or whatever the ll driver does). That's the point of being raw, isn't > it? > > If I'm right with that, would it make sense to go back to kzalloc(65)? > > If I'm wrong, then if you agree, I'll default to: > > hid_hw_raw_request(steam->hid_dev, 0x00, > buf, hid_report_len(r) + 1, /* count the request number */ > HID_FEATURE_REPORT, HID_REQ_GET_REPORT); > > Then, if hid_report_len() is ever updated to return the +1, this one > should be removed. And even if it is not, we still have +6 extra bytes > from hid_alloc_report_buf(), so no real harm done. I am fine with your analysis except for the last point :) We need all 7 extra bytes, not 6 (in implement()). However, as you said, implement() is not used in the low_level transport functions, so there is no point bike shedding for ages. I'd say please stick to hid_report_alloc_buf (maybe add a comment about the missing report ID added by usb), and use hid_report_len(r) + 1 while calling hid_hw_raw_request(). This way, we can always fix the behavior later and have something which will not break. How does that sound? Cheers, Benjamin > > Regards. > Rodrigo