From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: ARC-Seal: i=1; a=rsa-sha256; t=1516404276; cv=none; d=google.com; s=arc-20160816; b=YOe8nrFsKsh1S0tdd90+LArfrC+Vsmur45MaK1wB348DXZ5ihhKQSwflepgjWaMTvN nJMBPYTVGlEJp3yKHM1u92BwGGNoyM89BKmD/XZdXK0jwqgcOUcSJEmdQIdmI+XQ0jlt TtHrVmmNRtOEpOZuQ6G0oNpb8NmQRS3HLEymaP0JuS/WlaSOR91VP5l/QTRhPndh9U9l 3h41gxrAYIXnSNWVPTC8bBa7Mj2PCWkftaGrc1+cE3Ob5VnxT/NBqumzr9gDN7tkxxYz PA1YGk5Bfnf/CMkb8mCaTU2QJgNwVZGA3hzAYe7zpIUdBXnAwMZd12q6/VNkFFnrG5si Qopw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:dkim-signature :arc-authentication-results; bh=pglPq6xj5XuwgU959IFLSXvYLhulqBHyfuZo3BaNY2U=; b=wyHlADAvrbv7nnbKbJeNh1f9XGL9XvLQznITeXBOVLusNn38mlv/fEf1e4E5+mANGl 0wlqqukRtGC9ujmQqdMzHhjdZ2gDW1058hp4vlrbTgU+6cpRtrinCbLXJZXlXy0ODhWQ OqItNt3Qb/gh0jvwAiEuWm4D2pTvpHv2tECfpB41iZvAzFs9N3i4l+dl5yYyH38ui4Iq 0DKkmPPWPjo3EdoLgss/0XWJ9INyNw6zLvwOCf7jv7ii9VJt0Ggq/3hPO2qDMdVAu6gg T9sS6eXMw5QHS8lQZTRVRoErn9W7Z+9EykJWIBfx6c5i6zI3WqjEuDeOOmUF42+LS/xD +M0g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=SuDXCtet; spf=pass (google.com: domain of dmitry.torokhov@gmail.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=dmitry.torokhov@gmail.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gmail.com Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=SuDXCtet; spf=pass (google.com: domain of dmitry.torokhov@gmail.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=dmitry.torokhov@gmail.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gmail.com X-Google-Smtp-Source: AH8x225mR5Y/0+3NbOHUdDjT+3lIgfLt1pMUWrldP3Dgf+e7XY8frgbavospeKNVPuaN6HbE7BR3dg== Date: Fri, 19 Jan 2018 15:24:32 -0800 From: Dmitry Torokhov To: Marcus Folkesson Cc: Jonathan Corbet , Tomohiro Yoshidomi , David Herrmann , Philippe Ombredanne , Kate Stewart , Greg Kroah-Hartman , linux-input@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3] input: pxrc: new driver for PhoenixRC Flight Controller Adapter Message-ID: <20180119232432.xygulmwpkw3vogls@dtor-ws> References: <20180113201532.26016-1-marcus.folkesson@gmail.com> <20180116231625.e3sgum667xfuumx4@dtor-ws> <20180117135840.GA10949@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180117135840.GA10949@gmail.com> User-Agent: NeoMutt/20170609 (1.8.3) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1589509666987149281?= X-GMAIL-MSGID: =?utf-8?q?1590065130396947015?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Wed, Jan 17, 2018 at 02:58:40PM +0100, Marcus Folkesson wrote: > Hello Dmitry, > > On Tue, Jan 16, 2018 at 03:16:25PM -0800, Dmitry Torokhov wrote: > > Hi Marcus, > > > > On Sat, Jan 13, 2018 at 09:15:32PM +0100, Marcus Folkesson wrote: > > > This driver let you plug in your RC controller to the adapter and > > > use it as input device in various RC simulators. > > > > > > Signed-off-by: Marcus Folkesson > > > --- > > > v3: > > > - Use RUDDER and MISC instead of TILT_X and TILT_Y > > > - Drop kref and anchor > > > - Rework URB handling > > > - Add PM support > > > > How did you test the PM support? By default the autopm is disabled on > > USB devices; you need to enable it by writing to sysfs (I believe you > > need to 'echo "auto" > /sys/bus/usb//power/control) and see if > > it gets autosuspended when not in use and resumed after you start > > interacting with it. > > The test I've done is simply reading from the input device and then call > `pm-suspend`. > It works, suspend is called and reset_resume() will submit the URB > again. Without the PM code, the application did not read any events upon > resume. We are talking about different things. You are testing system suspend, whereas I was talking about runtime suspend (that's what usb_autopm_get_interface() and friends does). It is disabled by default and you need to enable it by writing into sysfs as I mentioned above. Then, after a few seconds of not touching the device you should see the USB interface going into low power state and the device shoudl correctly implement remote wakeup signal to wake up the host controller/port when user touches it. If the device does not implement this correctly, then after suspending it will "die". > > However, I found another tricky part. > If I enable autosuspend (as you suggest) it will suspend when noone is > using the device. Good. > > But when someone is opening the device, input_dev->users is counted up > to 1 before resume() is called. > Is this intended? > > This code (from resume()) will therefor allways submit the URB: > > if (input_dev->users && usb_submit_urb(pxrc->urb, GFP_NOIO) < 0) > > > Then open() is called and fails because the urb is allready submitted. > > input_dev->users is only incremented in input.c:input_open_device() what > I can tell? It is intended, but I guess we should not be using input_dev->users in resume(), but rather have a local flag in your driver structure trhat you update at the right time (i.e. after you submit USB in pxrc_open()). I suppose we need the same fix in synaptics_usb.c... > > I will move the submitting code to reset_resume() instead. You need both resume() and reset_resume(), they are called in different cases and you need to restart IO in both cases. Thanks. -- Dmitry