Re: [PATCH 1/2] hid-multitouch: work on default classes and renaming of classes

On Fri, Jan 28, 2011 at 18:18, Henrik Rydberg [off-list ref] wrote:

Hi Benjamin,

quoted

The safest quirk for a device (the one that works out of the box for
most of them) is the MT_QUIRK_NOT_SEEN_MEANS_UP. Indeed, it does not
make any assumption on the device. When adding a new device, we can
easily test it against MT_CLS_DEFAULT, and then optimize it with other
quirks: that's why no device use MT_CLS_DEFAULT right now.

This patch introduce also MT_CLS_DUAL_DEFAULT which has the same purpose
than MT_CLS_DEFAULT, but for dual touch panels.

But it is used anywhere?

Hi Henrik,

In it's current form, no. I often rely on it to make a quick patch
(just adding the ids in hid-ids, hid-core and hid-multitouch) to test
a new (dual touch) device. I thought it would be useful to be
mainstream.

quoted

Finally, the patch renames MT_CLS_DUAL1 to MT_CLS_DUAL_INRANGE_CONTACTID
and MT_CLS_DUAL2 to MT_CLS_DUAL_INRANGE_CONTACTNUMBER for better
readability.

Signed-off-by: Benjamin Tissoires <redacted>
---

The patch description and the content lacks a certain
distinctness. Please single out janitory actions into a separate
patch, or, if possible, skip it altogether.

ok, will do

quoted

 drivers/hid/hid-multitouch.c |   25 +++++++++++++++----------
 1 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 07d3183..ffe2a76 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c

@@ -24,6 +24,7 @@

 MODULE_AUTHOR("Stephane Chatty [off-list ref]");
+MODULE_AUTHOR("Benjamin Tissoires [off-list ref]");

Adding this is fine, based on your prior contributions. Perhaps it
should be motivated as such in a separate patch instead.

ok

quoted

 MODULE_DESCRIPTION("HID multitouch panels");
 MODULE_LICENSE("GPL");

@@ -65,10 +66,11 @@ struct mt_class {

 };

 /* classes of device behavior */
-#define MT_CLS_DEFAULT       1
-#define MT_CLS_DUAL1 2
-#define MT_CLS_DUAL2 3
-#define MT_CLS_CYPRESS       4
+#define MT_CLS_DEFAULT                               1
+#define MT_CLS_DUAL_DEFAULT                  2
+#define MT_CLS_DUAL_INRANGE_CONTACTID                3
+#define MT_CLS_DUAL_INRANGE_CONTACTNUMBER    4
+#define MT_CLS_CYPRESS                               10

There is no need to change the numbering for unchanged names.

I just wanted to keep device-specific at the end of the list. Hence
the 10 to keep a little space between generic and devices:
MT_CLS_DUAL_CONFIDENCE_CONTACT* are coming...

quoted

 /*
  * these device-dependent functions determine what slot corresponds

@@ -104,13 +106,16 @@ static int find_slot_from_contactid(struct mt_device *td)

 struct mt_class mt_classes[] = {
      { .name = MT_CLS_DEFAULT,
-             .quirks = MT_QUIRK_VALID_IS_INRANGE,
+             .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP,
              .maxcontacts = 10 },
-     { .name = MT_CLS_DUAL1,
+     { .name = MT_CLS_DUAL_DEFAULT,
+             .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP,
+             .maxcontacts = 2 },
+     { .name = MT_CLS_DUAL_INRANGE_CONTACTID,
              .quirks = MT_QUIRK_VALID_IS_INRANGE |
                      MT_QUIRK_SLOT_IS_CONTACTID,
              .maxcontacts = 2 },
-     { .name = MT_CLS_DUAL2,
+     { .name = MT_CLS_DUAL_INRANGE_CONTACTNUMBER,
              .quirks = MT_QUIRK_VALID_IS_INRANGE |
                      MT_QUIRK_SLOT_IS_CONTACTNUMBER,
              .maxcontacts = 2 },

@@ -466,15 +471,15 @@ static const struct hid_device_id mt_devices[] = {

                      USB_DEVICE_ID_CYPRESS_TRUETOUCH) },

      /* GeneralTouch panel */
-     { .driver_data = MT_CLS_DUAL2,
+     { .driver_data = MT_CLS_DUAL_INRANGE_CONTACTNUMBER,
              HID_USB_DEVICE(USB_VENDOR_ID_GENERAL_TOUCH,
                      USB_DEVICE_ID_GENERAL_TOUCH_WIN7_TWOFINGERS) },

      /* PixCir-based panels */
-     { .driver_data = MT_CLS_DUAL1,
+     { .driver_data = MT_CLS_DUAL_INRANGE_CONTACTID,
              HID_USB_DEVICE(USB_VENDOR_ID_HANVON,
                      USB_DEVICE_ID_HANVON_MULTITOUCH) },
-     { .driver_data = MT_CLS_DUAL1,
+     { .driver_data = MT_CLS_DUAL_INRANGE_CONTACTID,
              HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
                      USB_DEVICE_ID_CANDO_PIXCIR_MULTI_TOUCH) },

--
1.7.3.4

Thanks,
Henrik

Thanks for the review,
Benjamin
--
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