From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dmitry Torokhov Subject: Re: [PATCH 1/2] Input: soc_button_array - Set input device name Date: Mon, 23 Jan 2017 14:10:44 -0800 Message-ID: <20170123221044.GA34769@dtor-ws> References: <20170109175707.12854-1-hdegoede@redhat.com> <20170121191328.GC36944@dtor-ws> <9dd3f04e-83e0-edb9-2296-ef6ea80c5adc@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mail-pf0-f194.google.com ([209.85.192.194]:33614 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751337AbdAWWKr (ORCPT ); Mon, 23 Jan 2017 17:10:47 -0500 Received: by mail-pf0-f194.google.com with SMTP id e4so10709638pfg.0 for ; Mon, 23 Jan 2017 14:10:47 -0800 (PST) Content-Disposition: inline In-Reply-To: <9dd3f04e-83e0-edb9-2296-ef6ea80c5adc@redhat.com> Sender: linux-input-owner@vger.kernel.org List-Id: linux-input@vger.kernel.org To: Hans de Goede Cc: "russianneuromancer @ ya . ru" , Gregor Riepl , "linux-input@vger.kernel.org" On Sun, Jan 22, 2017 at 11:10:31AM +0100, Hans de Goede wrote: > Hi, > > On 22-01-17 11:00, Dmitry Torokhov wrote: > >On Sun, Jan 22, 2017 at 12:49 AM, Hans de Goede wrote: > >>Hi, > >> > >>On 21-01-17 20:13, Dmitry Torokhov wrote: > >>> > >>>On Mon, Jan 09, 2017 at 06:57:06PM +0100, Hans de Goede wrote: > >>>> > >>>>On some tablets using the soc_button_array driver the buttons do not > >>>>follow the standard home, power, volume_up, volume_down, rotation_lock > >>>>button order as published by Microsoft. > >>>> > >>>>We can use the existing udev hwdb mechanism to fix this up, but then > >>>>the created devices must have a unique name, therefor this commit adds > >>>>a unique name for the 2 created gpio-keys input devices. > >>> > >>> > >>>Why does it have to have unique name? You should be able to match on > >>>other input device properties, for example ATTR{capabilities/ev} or > >>>ATTR{capabilities/keys} to identify the device you want to adjust. > >> > >> > >>hwdb entries do not have access to full udev data, basically there > >>are 2 match formats: > >> > >># Supported hardware matches are: > >># - Generic input devices match: > >># evdev:input:bZZZZvYYYYpXXXXeWWWW-VVVV > >># This matches on the kernel modalias of the input-device, mainly: > >># ZZZZ is the bus-id (see /usr/include/linux/input.h BUS_*), YYYY, XXXX > >>and > >># WWW are the 4-digit hex uppercase vendor, product and version ID and > >>VVVV > >># is an arbitrary length input-modalias describing the device > >>capabilities. > >># The vendor, product and version ID for a device node "eventX" is listed > >># in /sys/class/input/eventX/device/id. > >># > >># - Input driver device name and DMI data match: > >># evdev:name::dmi:bvn*:bvr*:bd*:svn:pn* > >># is the name device specified by the > >># driver, is the firmware-provided string exported > >># by the kernel DMI modalias, see /sys/class/dmi/id/modalias > >> > >> > >>Since we want to match on DMI info we need to use the second, and > >>the info you are referring to is not available here. > > > >Well, you can either teach hwdb new tricks or mangle the name in udev > >rule. As far as I can see the original invocation is: > > > ># device matching the input device name and the machine's DMI data > >KERNELS=="input*", IMPORT{builtin}="hwdb > >'evdev:name:$attr{name}:$attr{[dmi/id]modalias}'", \ > > RUN{builtin}+="keyboard", GOTO="evdev_end" > > > >You can add a similar rule that also looks at ATTR{whatever}, but > >instead of using "name:$attr{name}" you can use whatever string you > >want. > > > >There is no need to change kernel, it already exports all necessary data. > > Ah, come one, requiring a custom udev rule for this is a pain, where as OK, do not require custom property. Provide an extended one: # device matching the input device properties and the machine's DMI data KERNELS=="input*", IMPORT{builtin}="hwdb 'evdev:name:$attr{name}:phys:$attr{phys}:ev:$attr{capabilities/ev}:$attr{[dmi/id]modalias}'", \ RUN{builtin}+="keyboard" and either migrate old keymaps to the extended one, or keep both. In the end, there is nothing special in 'evdev:name:...' prefix, it matches across all lines of hwdb with whatever is provided in the rule. > this is really easy to fix on the kernel side. Even if changing the kernel seems most convenient for you it does not mean it is the right approach. > > Besides that, the way soc_button_array works is that we currently end > up with 2 identical named (gpio_keys) input devices which is all sorts > of inconvenient, e.g. during testing the setkeycode support I had > to guess which was which when invoking evemu-record to test, they > will have the same name in "xinput list", etc. > > Input device names really should be unique where ever possible, for > various reasons, and here we can easily make them unique. We never provided any guarantees about uniqueness of names of input devices and I do not think we should start now. If you plug 2 same USB keyboards you'll get 2 devices with same names. Same goes for mice, tablets, etc. I do not see why SOC buttons should be different. Thanks. -- Dmitry