* [PATCH 1/2] Input: soc_button_array - Set input device name @ 2017-01-09 17:57 Hans de Goede 2017-01-09 17:57 ` [PATCH 2/2] Input: soc_button_array - Debounce the buttons Hans de Goede 2017-01-21 19:13 ` [PATCH 1/2] Input: soc_button_array - Set input device name Dmitry Torokhov 0 siblings, 2 replies; 11+ messages in thread From: Hans de Goede @ 2017-01-09 17:57 UTC (permalink / raw) To: Dmitry Torokhov Cc: Hans de Goede, russianneuromancer @ ya . ru, Gregor Riepl, linux-input 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. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/input/misc/soc_button_array.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c index 9bc1b20..d2e5186 100644 --- a/drivers/input/misc/soc_button_array.c +++ b/drivers/input/misc/soc_button_array.c @@ -113,6 +113,11 @@ soc_button_device_create(struct platform_device *pdev, gpio_keys_pdata->nbuttons = n_buttons; gpio_keys_pdata->rep = autorepeat; + if (autorepeat) + gpio_keys_pdata->name = "SoC Button Array (autorepeat buttons)"; + else + gpio_keys_pdata->name = "SoC Button Array"; + pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO); if (!pd) { error = -ENOMEM; -- 2.9.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] Input: soc_button_array - Debounce the buttons 2017-01-09 17:57 [PATCH 1/2] Input: soc_button_array - Set input device name Hans de Goede @ 2017-01-09 17:57 ` Hans de Goede 2017-01-21 19:14 ` Dmitry Torokhov 2017-01-21 19:13 ` [PATCH 1/2] Input: soc_button_array - Set input device name Dmitry Torokhov 1 sibling, 1 reply; 11+ messages in thread From: Hans de Goede @ 2017-01-09 17:57 UTC (permalink / raw) To: Dmitry Torokhov Cc: Hans de Goede, russianneuromancer @ ya . ru, Gregor Riepl, linux-input The soc_button_array driver was initializing (kzalloc) the debounce_interval value to 0, leading to no debouncing at all, while the buttons are simple mechanical switches. This commit sets debounce_interval to 50ms to avoid spurious button press reports both on press and release of the button. Note 50ms may seem like a lot but soc_button_array is typically used with cheap tablets, with not so great buttons. I tried 10ms on my tablet and it is not enough, where as 50ms works well. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/input/misc/soc_button_array.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c index d2e5186..e53b0a1 100644 --- a/drivers/input/misc/soc_button_array.c +++ b/drivers/input/misc/soc_button_array.c @@ -101,6 +101,8 @@ soc_button_device_create(struct platform_device *pdev, gpio_keys[n_buttons].active_low = 1; gpio_keys[n_buttons].desc = info->name; gpio_keys[n_buttons].wakeup = info->wakeup; + /* These devices often use cheap buttons, use 50 ms debounce */ + gpio_keys[n_buttons].debounce_interval = 50; n_buttons++; } -- 2.9.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Input: soc_button_array - Debounce the buttons 2017-01-09 17:57 ` [PATCH 2/2] Input: soc_button_array - Debounce the buttons Hans de Goede @ 2017-01-21 19:14 ` Dmitry Torokhov 0 siblings, 0 replies; 11+ messages in thread From: Dmitry Torokhov @ 2017-01-21 19:14 UTC (permalink / raw) To: Hans de Goede; +Cc: russianneuromancer @ ya . ru, Gregor Riepl, linux-input On Mon, Jan 09, 2017 at 06:57:07PM +0100, Hans de Goede wrote: > The soc_button_array driver was initializing (kzalloc) the > debounce_interval value to 0, leading to no debouncing at all, > while the buttons are simple mechanical switches. > > This commit sets debounce_interval to 50ms to avoid spurious button > press reports both on press and release of the button. Note 50ms may > seem like a lot but soc_button_array is typically used with cheap > tablets, with not so great buttons. I tried 10ms on my tablet and it > is not enough, where as 50ms works well. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Applied, thank you. > --- > drivers/input/misc/soc_button_array.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c > index d2e5186..e53b0a1 100644 > --- a/drivers/input/misc/soc_button_array.c > +++ b/drivers/input/misc/soc_button_array.c > @@ -101,6 +101,8 @@ soc_button_device_create(struct platform_device *pdev, > gpio_keys[n_buttons].active_low = 1; > gpio_keys[n_buttons].desc = info->name; > gpio_keys[n_buttons].wakeup = info->wakeup; > + /* These devices often use cheap buttons, use 50 ms debounce */ > + gpio_keys[n_buttons].debounce_interval = 50; > n_buttons++; > } > > -- > 2.9.3 > -- Dmitry ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Input: soc_button_array - Set input device name 2017-01-09 17:57 [PATCH 1/2] Input: soc_button_array - Set input device name Hans de Goede 2017-01-09 17:57 ` [PATCH 2/2] Input: soc_button_array - Debounce the buttons Hans de Goede @ 2017-01-21 19:13 ` Dmitry Torokhov 2017-01-22 8:49 ` Hans de Goede 1 sibling, 1 reply; 11+ messages in thread From: Dmitry Torokhov @ 2017-01-21 19:13 UTC (permalink / raw) To: Hans de Goede; +Cc: russianneuromancer @ ya . ru, Gregor Riepl, linux-input 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. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/input/misc/soc_button_array.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c > index 9bc1b20..d2e5186 100644 > --- a/drivers/input/misc/soc_button_array.c > +++ b/drivers/input/misc/soc_button_array.c > @@ -113,6 +113,11 @@ soc_button_device_create(struct platform_device *pdev, > gpio_keys_pdata->nbuttons = n_buttons; > gpio_keys_pdata->rep = autorepeat; > > + if (autorepeat) > + gpio_keys_pdata->name = "SoC Button Array (autorepeat buttons)"; > + else > + gpio_keys_pdata->name = "SoC Button Array"; > + > pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO); > if (!pd) { > error = -ENOMEM; > -- > 2.9.3 > Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Input: soc_button_array - Set input device name 2017-01-21 19:13 ` [PATCH 1/2] Input: soc_button_array - Set input device name Dmitry Torokhov @ 2017-01-22 8:49 ` Hans de Goede 2017-01-22 10:00 ` Dmitry Torokhov 0 siblings, 1 reply; 11+ messages in thread From: Hans de Goede @ 2017-01-22 8:49 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: russianneuromancer @ ya . ru, Gregor Riepl, linux-input 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:<input device name>:dmi:bvn*:bvr*:bd*:svn<vendor>:pn* # <input device name> is the name device specified by the # driver, <vendor> 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. Regards, Hans > >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/input/misc/soc_button_array.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c >> index 9bc1b20..d2e5186 100644 >> --- a/drivers/input/misc/soc_button_array.c >> +++ b/drivers/input/misc/soc_button_array.c >> @@ -113,6 +113,11 @@ soc_button_device_create(struct platform_device *pdev, >> gpio_keys_pdata->nbuttons = n_buttons; >> gpio_keys_pdata->rep = autorepeat; >> >> + if (autorepeat) >> + gpio_keys_pdata->name = "SoC Button Array (autorepeat buttons)"; >> + else >> + gpio_keys_pdata->name = "SoC Button Array"; >> + >> pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO); >> if (!pd) { >> error = -ENOMEM; >> -- >> 2.9.3 >> > > Thanks. > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Input: soc_button_array - Set input device name 2017-01-22 8:49 ` Hans de Goede @ 2017-01-22 10:00 ` Dmitry Torokhov 2017-01-22 10:10 ` Hans de Goede 0 siblings, 1 reply; 11+ messages in thread From: Dmitry Torokhov @ 2017-01-22 10:00 UTC (permalink / raw) To: Hans de Goede; +Cc: russianneuromancer @ ya . ru, Gregor Riepl, linux-input On Sun, Jan 22, 2017 at 12:49 AM, Hans de Goede <hdegoede@redhat.com> 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:<input device name>:dmi:bvn*:bvr*:bd*:svn<vendor>:pn* > # <input device name> is the name device specified by the > # driver, <vendor> 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. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Input: soc_button_array - Set input device name 2017-01-22 10:00 ` Dmitry Torokhov @ 2017-01-22 10:10 ` Hans de Goede 2017-01-23 21:17 ` Hans de Goede 2017-01-23 22:10 ` Dmitry Torokhov 0 siblings, 2 replies; 11+ messages in thread From: Hans de Goede @ 2017-01-22 10:10 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: russianneuromancer @ ya . ru, Gregor Riepl, linux-input Hi, On 22-01-17 11:00, Dmitry Torokhov wrote: > On Sun, Jan 22, 2017 at 12:49 AM, Hans de Goede <hdegoede@redhat.com> 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:<input device name>:dmi:bvn*:bvr*:bd*:svn<vendor>:pn* >> # <input device name> is the name device specified by the >> # driver, <vendor> 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 this is really easy to fix on the kernel side. 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. Regards, Hans ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Input: soc_button_array - Set input device name 2017-01-22 10:10 ` Hans de Goede @ 2017-01-23 21:17 ` Hans de Goede 2017-01-23 22:10 ` Dmitry Torokhov 1 sibling, 0 replies; 11+ messages in thread From: Hans de Goede @ 2017-01-23 21:17 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: russianneuromancer @ ya . ru, Gregor Riepl, linux-input Hi Dmitry, On 22-01-17 11:10, 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 <hdegoede@redhat.com> 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:<input device name>:dmi:bvn*:bvr*:bd*:svn<vendor>:pn* >>> # <input device name> is the name device specified by the >>> # driver, <vendor> 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 > this is really easy to fix on the kernel side. > > 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. Dmitry, can you please respond to the above ? I still believe that having a unique name for the 2 devices is a good idea, see above for my reasons. Can you please merge this patch ? I would like to send matching patches to the hwdb upstream to fix home and power being swapped on some devices but first this needs to be resolved... Regards, Hans ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Input: soc_button_array - Set input device name 2017-01-22 10:10 ` Hans de Goede 2017-01-23 21:17 ` Hans de Goede @ 2017-01-23 22:10 ` Dmitry Torokhov 2017-01-23 22:14 ` Dmitry Torokhov 2017-02-12 12:36 ` Hans de Goede 1 sibling, 2 replies; 11+ messages in thread From: Dmitry Torokhov @ 2017-01-23 22:10 UTC (permalink / raw) To: Hans de Goede; +Cc: russianneuromancer @ ya . ru, Gregor Riepl, linux-input 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 <hdegoede@redhat.com> 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:<input device name>:dmi:bvn*:bvr*:bd*:svn<vendor>:pn* > >># <input device name> is the name device specified by the > >># driver, <vendor> 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Input: soc_button_array - Set input device name 2017-01-23 22:10 ` Dmitry Torokhov @ 2017-01-23 22:14 ` Dmitry Torokhov 2017-02-12 12:36 ` Hans de Goede 1 sibling, 0 replies; 11+ messages in thread From: Dmitry Torokhov @ 2017-01-23 22:14 UTC (permalink / raw) To: Hans de Goede; +Cc: russianneuromancer @ ya . ru, Gregor Riepl, linux-input On Mon, Jan 23, 2017 at 02:10:44PM -0800, Dmitry Torokhov wrote: > 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 <hdegoede@redhat.com> 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:<input device name>:dmi:bvn*:bvr*:bd*:svn<vendor>:pn* > > >># <input device name> is the name device specified by the > > >># driver, <vendor> 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. Forgot to add: for ordinary users, distinction between between autorepeat and not-autorepeat SOC buttons devices is absolutely uninteresting. They could care less. You just want something different about them to trigger hwdb match, and we already have that, but not in name. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Input: soc_button_array - Set input device name 2017-01-23 22:10 ` Dmitry Torokhov 2017-01-23 22:14 ` Dmitry Torokhov @ 2017-02-12 12:36 ` Hans de Goede 1 sibling, 0 replies; 11+ messages in thread From: Hans de Goede @ 2017-02-12 12:36 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: russianneuromancer @ ya . ru, Gregor Riepl, linux-input Hi, On 23-01-17 23:10, Dmitry Torokhov wrote: > 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 <hdegoede@redhat.com> 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:<input device name>:dmi:bvn*:bvr*:bd*:svn<vendor>:pn* >>>> # <input device name> is the name device specified by the >>>> # driver, <vendor> 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. Ok, I've send a pull-req to the systemd maintainers with a new rule for extended input matches + a hwdb entry for the tablet with the issue using the new extended rule. Regards, Hans ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-02-12 12:36 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-09 17:57 [PATCH 1/2] Input: soc_button_array - Set input device name Hans de Goede 2017-01-09 17:57 ` [PATCH 2/2] Input: soc_button_array - Debounce the buttons Hans de Goede 2017-01-21 19:14 ` Dmitry Torokhov 2017-01-21 19:13 ` [PATCH 1/2] Input: soc_button_array - Set input device name Dmitry Torokhov 2017-01-22 8:49 ` Hans de Goede 2017-01-22 10:00 ` Dmitry Torokhov 2017-01-22 10:10 ` Hans de Goede 2017-01-23 21:17 ` Hans de Goede 2017-01-23 22:10 ` Dmitry Torokhov 2017-01-23 22:14 ` Dmitry Torokhov 2017-02-12 12:36 ` Hans de Goede
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.