lkml.org 
[lkml]   [2021]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 0/1] Do not map BTN_RIGHT/MIDDLE on buttonpads
Hi José,

On Tue, Nov 23, 2021 at 8:12 PM José Expósito <jose.exposito89@gmail.com> wrote:
>
> Hi all,
>
> Historically, libinput has relayed on the INPUT_PROP_BUTTONPAD property
> to detect buttonpads.
>
> Since buttonpads are expected to have only one button (BTN_LEFT),
> recently we added a new rule to detect buttonpads: Where a touchpad
> maps the BTN_RIGHT bit, libinput assumes it is NOT a buttonpad.
>
> However, this change leaded to several false possitives, so we ended up
> reverting it. For more context:
> https://gitlab.freedesktop.org/libinput/libinput/-/issues/704
>
> And for a full list of affected hardware, HID reports and bug reports
> please see:
> https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/726
>
> My understanding is that buttonpads should not map BTN_RIGHT and/or
> BTN_MIDDLE and to avoid it I would like to fix the required drivers.

As long as udev intrinsic is happy with it (and it correctly tags the
touchpad as ID_INPUT_something), I'm fine with it.

Also, you might want to point at the specification regarding button
pads: https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-windows-precision-touchpad-collection#device-capabilities-feature-report

The way I read it: if the device exports the Button type value
feature, and it is 0 or 1 (click-pad or pressure-pad), there should
not be discrete buttons.

>
> One option to fix it (this patch) is to clear the bits that might have
> been added because of the HID descriptor on every driver.
> However, since this code will be common to all drivers, I would like to
> ask if you consider it worth it to add a function to handle adding
> properties.
>
> A function similar to input_set_capability but for props could be added
> in input.h/c:
>
> /**
> * input_set_property - add a property to the device
> * @dev: device to add the property to
> * @property: type of the property (INPUT_PROP_POINTER, INPUT_PROP_DIRECT...)
> *
> * In addition to setting up corresponding bit in dev->propbit the function
> * might add or remove related capabilities.
> */
> void input_set_property(struct input_dev *dev, unsigned int property)
> {
> switch (property) {
> case INPUT_PROP_POINTER:
> case INPUT_PROP_DIRECT:
> case INPUT_PROP_SEMI_MT:
> case INPUT_PROP_TOPBUTTONPAD:
> case INPUT_PROP_POINTING_STICK:
> case INPUT_PROP_ACCELEROMETER:
> break;
>
> case INPUT_PROP_BUTTONPAD:
> input_set_capability(dev, EV_KEY, BTN_LEFT);
> __clear_bit(BTN_RIGHT, dev->keybit);
> __clear_bit(BTN_MIDDLE, dev->keybit);
> break;
>
> default:
> pr_err("%s: unknown property %u\n", __func__, property);
> dump_stack();
> return;
> }
>
> __set_bit(property, dev->propbit);
> }
> EXPORT_SYMBOL(input_set_property);
>
>
> Which approach do you think is the best?

I think it depends if you plan on fixing just hid-multitouch or the others.
If you have more than one driver, then yes, adding a new symbol in
hid-input.c makes sense. If not, then you are just exposing a new
function we won't know if there are users and we won't be able to
change without care.

Cheers,
Benjamin

>
> Thank you very much in advance,
> Jose
>
>
> José Expósito (1):
> HID: multitouch: only map BTN_LEFT on buttonpads
>
> drivers/hid/hid-multitouch.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> --
> 2.25.1
>

\
 
 \ /
  Last update: 2021-11-24 10:40    [W:0.722 / U:0.996 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site