From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752796AbcEWRTK (ORCPT ); Mon, 23 May 2016 13:19:10 -0400 Received: from mout01.posteo.de ([185.67.36.65]:57326 "EHLO mout01.posteo.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751852AbcEWRTI (ORCPT ); Mon, 23 May 2016 13:19:08 -0400 Subject: Re: [PATCH v3] input: tablet: add Pegasus Notetaker tablet driver To: Oliver Neukum References: <1464003592-24043-1-git-send-email-martink@posteo.de> <1464006414.12181.53.camel@suse.com> <5742FB0F.5070508@posteo.de> <1464008447.12181.58.camel@suse.com> Cc: dmitry.torokhov@gmail.com, kernel-testers@vger.kernel.org, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org From: Martin Kepplinger Message-ID: <57433B85.1050301@posteo.de> Date: Mon, 23 May 2016 19:19:01 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.8.0 MIME-Version: 1.0 In-Reply-To: <1464008447.12181.58.camel@suse.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 2016-05-23 um 15:00 schrieb Oliver Neukum: > On Mon, 2016-05-23 at 14:43 +0200, Martin Kepplinger wrote: >> Am 2016-05-23 um 14:26 schrieb Oliver Neukum: >>> On Mon, 2016-05-23 at 13:39 +0200, Martin Kepplinger wrote: >>> >>>> It's *really* fun to use as an input tablet though! So let's support this >>>> for everybody. >>> >>> Hi, >>> >>> I am afraid there are a few issues. >> >> Thanks for having a look, comments below. >> >>> >>> 1. Why the kernel thread? >> >> I provide switching to the xy tablet mode by pressing a button during >> runtime. So usb_control_msg() is called from the thread, not the >> interrupt routine. >> >> We don't get in the way of any user app. If we leave that functionality >> out, users may have to un- and replug in the device after using >> userspace apps that transfer data or the like. > > The functionality is OK, but the implementation, though creative, > is buggy. > > a - multiple devices will fail > b - you have a race condition that will fail to wake the thread > if it is still running when the button is pressed again > > Kernel threads are wasteful. Using the normal system work queue might be > much easier You're right. I'll definitely change this. > >>> 2. This driver has questionable power management. >> >> I think I do it how others do it. Suspend/resume works. Anything >> specific required here? > > Runtime PM is missing. There is no way to turn the device off. So like any other mouse or input tablet we don't use runtime PM. In case the Pen is out of range or off, we just don't handle URBs and do nothing. ... but I may miss something, so in this case please get back to me. thanks martin > > Regards > Oliver > >