| Message ID | 1311545261-34067-2-git-send-email-jeffbrown@android.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
Le 25 juil. 2011 à 00:07, jeffbrown@android.com a écrit : > From: Jeff Brown <jeffbrown@google.com> > > This change fixes two problems. > > First, it ensures that the hid-multitouch driver does not incorrectly > map GenericDesktop usages that are intended for other applications, > such as a Mouse. > > Second, it sets the appropriate input properties so that user-space > can distinguish TouchScreen devices (INPUT_PROP_DIRECT) from > TouchPad devices (INPUT_PROP_POINTER) and configure them accordingly. > > Signed-off-by: jeffbrown@android.com > --- > drivers/hid/hid-multitouch.c | 10 ++++++++++ > 1 files changed, 10 insertions(+), 0 deletions(-) > > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c > index 58d0e7a..4ee21ac 100644 > --- a/drivers/hid/hid-multitouch.c > +++ b/drivers/hid/hid-multitouch.c > @@ -213,6 +213,16 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, > struct mt_class *cls = td->mtclass; > __s32 quirks = cls->quirks; > > + /* Only map fields from TouchScreen or TouchPad collections. > + * We need to ignore fields that belong to other collections > + * such as Mouse that might have the same GenericDesktop usages. */ > + if (field->application == HID_DG_TOUCHSCREEN) > + set_bit(INPUT_PROP_DIRECT, hi->input->propbit); > + else if (field->application == HID_DG_TOUCHPAD) > + set_bit(INPUT_PROP_POINTER, hi->input->propbit); > + else > + return 0; > + > switch (usage->hid & HID_USAGE_PAGE) { > > case HID_UP_GENDESK: > -- > 1.7.0.4 > Looks clever to me. Still, in principle we should check that all (or at least most) known devices fill the application field properly. I'll try to have a look at the report descriptors we have here. St. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Jeff, I like the patch as it partially solves the problems left by b84bd27fe70206f9253c395958134e4e4b7e55f0 (fix broken egalax). The previous patch mapped buttons but now it should be ok. Though, we still have the problem of the distinction between stylus and finger. It would worth the effort to know if reverting b84bd27fe70206f9253c395958134e4e4b7e55f0 and apply this one instead still works for eGalax 480d owners (and mosart too IIRC). On Mon, Jul 25, 2011 at 11:16, Stéphane Chatty <chatty@enac.fr> wrote: > > Le 25 juil. 2011 à 00:07, jeffbrown@android.com a écrit : > >> From: Jeff Brown <jeffbrown@google.com> >> >> This change fixes two problems. >> >> First, it ensures that the hid-multitouch driver does not incorrectly >> map GenericDesktop usages that are intended for other applications, >> such as a Mouse. >> >> Second, it sets the appropriate input properties so that user-space >> can distinguish TouchScreen devices (INPUT_PROP_DIRECT) from >> TouchPad devices (INPUT_PROP_POINTER) and configure them accordingly. >> >> Signed-off-by: jeffbrown@android.com >> --- >> drivers/hid/hid-multitouch.c | 10 ++++++++++ >> 1 files changed, 10 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c >> index 58d0e7a..4ee21ac 100644 >> --- a/drivers/hid/hid-multitouch.c >> +++ b/drivers/hid/hid-multitouch.c >> @@ -213,6 +213,16 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, >> struct mt_class *cls = td->mtclass; >> __s32 quirks = cls->quirks; >> >> + /* Only map fields from TouchScreen or TouchPad collections. >> + * We need to ignore fields that belong to other collections >> + * such as Mouse that might have the same GenericDesktop usages. */ >> + if (field->application == HID_DG_TOUCHSCREEN) >> + set_bit(INPUT_PROP_DIRECT, hi->input->propbit); >> + else if (field->application == HID_DG_TOUCHPAD) >> + set_bit(INPUT_PROP_POINTER, hi->input->propbit); >> + else >> + return 0; Maybe I'm wrong, but 0 means that hid-input will decide what to do. Maybe we should return -1 instead, to be sure not handling this part. >> + >> switch (usage->hid & HID_USAGE_PAGE) { >> >> case HID_UP_GENDESK: >> -- >> 1.7.0.4 >> > > > Looks clever to me. Still, in principle we should check that all (or at least most) known devices fill the application field properly. I'll try to have a look at the report descriptors we have here. I checked (with some greps) the reports we have, and it seems that all the device we know provide this application. So it should be safe, but I'm not able to test it. Stéphane, if you have the time to test it on our lab's devices that would be great. Cheers, Benjamin > > St. > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le 25 juil. 2011 à 11:29, benjamin.tissoires@gmail.com a écrit : > Hi Jeff, > > I like the patch as it partially solves the problems left by > b84bd27fe70206f9253c395958134e4e4b7e55f0 (fix broken egalax). The > previous patch mapped buttons but now it should be ok. > Though, we still have the problem of the distinction between stylus and finger. > > It would worth the effort to know if reverting > b84bd27fe70206f9253c395958134e4e4b7e55f0 and apply this one instead > still works for eGalax 480d owners (and mosart too IIRC). > > > On Mon, Jul 25, 2011 at 11:16, Stéphane Chatty <chatty@enac.fr> wrote: >> >> Le 25 juil. 2011 à 00:07, jeffbrown@android.com a écrit : >> >>> From: Jeff Brown <jeffbrown@google.com> >>> >>> This change fixes two problems. >>> >>> First, it ensures that the hid-multitouch driver does not incorrectly >>> map GenericDesktop usages that are intended for other applications, >>> such as a Mouse. >>> >>> Second, it sets the appropriate input properties so that user-space >>> can distinguish TouchScreen devices (INPUT_PROP_DIRECT) from >>> TouchPad devices (INPUT_PROP_POINTER) and configure them accordingly. >>> >>> Signed-off-by: jeffbrown@android.com >>> --- >>> drivers/hid/hid-multitouch.c | 10 ++++++++++ >>> 1 files changed, 10 insertions(+), 0 deletions(-) >>> >>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c >>> index 58d0e7a..4ee21ac 100644 >>> --- a/drivers/hid/hid-multitouch.c >>> +++ b/drivers/hid/hid-multitouch.c >>> @@ -213,6 +213,16 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, >>> struct mt_class *cls = td->mtclass; >>> __s32 quirks = cls->quirks; >>> >>> + /* Only map fields from TouchScreen or TouchPad collections. >>> + * We need to ignore fields that belong to other collections >>> + * such as Mouse that might have the same GenericDesktop usages. */ >>> + if (field->application == HID_DG_TOUCHSCREEN) >>> + set_bit(INPUT_PROP_DIRECT, hi->input->propbit); >>> + else if (field->application == HID_DG_TOUCHPAD) >>> + set_bit(INPUT_PROP_POINTER, hi->input->propbit); >>> + else >>> + return 0; > > Maybe I'm wrong, but 0 means that hid-input will decide what to do. > Maybe we should return -1 instead, to be sure not handling this part. > >>> + >>> switch (usage->hid & HID_USAGE_PAGE) { >>> >>> case HID_UP_GENDESK: >>> -- >>> 1.7.0.4 >>> >> >> >> Looks clever to me. Still, in principle we should check that all (or at least most) known devices fill the application field properly. I'll try to have a look at the report descriptors we have here. > > I checked (with some greps) the reports we have, and it seems that all > the device we know provide this application. So it should be safe, but > I'm not able to test it. > Stéphane, if you have the time to test it on our lab's devices that > would be great. So far it works fine on our Cypress panel. I'll keep you posted about Lumio and Stantum as soon as we have finished compiling Linux 3.0. St. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 25, 2011 at 6:48 AM, Stéphane Chatty <chatty@enac.fr> wrote:
> So far it works fine on our Cypress panel. I'll keep you posted about Lumio and Stantum as soon as we have finished compiling Linux 3.0.
Actually, I'm wondering if we could merge hid-multitouch into hid-input.
Checking for an application usage of TouchScreen or TouchPad should be
pretty unambiguous. It ought to be possible to support most Windows 7
compatible HID multitouch devices out of the box without needing to
add them to any exception lists whatsoever, except for those few that
might need quirks.
Jeff.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Le 25 juil. 2011 à 20:08, Jeffrey Brown a écrit : > On Mon, Jul 25, 2011 at 6:48 AM, Stéphane Chatty <chatty@enac.fr> wrote: >> So far it works fine on our Cypress panel. I'll keep you posted about Lumio and Stantum as soon as we have finished compiling Linux 3.0. > > Actually, I'm wondering if we could merge hid-multitouch into hid-input. > > Checking for an application usage of TouchScreen or TouchPad should be > pretty unambiguous. It ought to be possible to support most Windows 7 > compatible HID multitouch devices out of the box without needing to > add them to any exception lists whatsoever, except for those few that > might need quirks. Yes, Benjamin and I have been working in this direction. We have experimental code that autodectects multitouch devices. It's not polished nor tested enough, but the number of requests for supporting new panels is a strong incentive for us to finish it :-) As far as I remember, this requires some restructuring in hid-input though. Cheers, St. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le 25 juil. 2011 à 11:29, benjamin.tissoires@gmail.com a écrit : > > I checked (with some greps) the reports we have, and it seems that all > the device we know provide this application. So it should be safe, but > I'm not able to test it. > Stéphane, if you have the time to test it on our lab's devices that > would be great. We checked with Stantum and Lumio panels, it works fine. Added to Benjamin's check in our database of report descriptors, it gives fairly good confidence that the change is innocuous (and actually beneficial). St. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 58d0e7a..4ee21ac 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -213,6 +213,16 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, struct mt_class *cls = td->mtclass; __s32 quirks = cls->quirks; + /* Only map fields from TouchScreen or TouchPad collections. + * We need to ignore fields that belong to other collections + * such as Mouse that might have the same GenericDesktop usages. */ + if (field->application == HID_DG_TOUCHSCREEN) + set_bit(INPUT_PROP_DIRECT, hi->input->propbit); + else if (field->application == HID_DG_TOUCHPAD) + set_bit(INPUT_PROP_POINTER, hi->input->propbit); + else + return 0; + switch (usage->hid & HID_USAGE_PAGE) { case HID_UP_GENDESK: