HID: hid-multitouch: minor fixes based on additional review

* amended Kconfig (PixCir and Hanvon are the same panel but with
  different name)
* insert field name in mt_class and retrieving it in mt_probe
* add 2 quirks: MT_QUIRK_VALID_IS_INRANGE, MT_QUIRK_VALID_IS_CONFIDENCE,
  in order to find the field "valid"
* inlined slot_is_contactid and slot_is_contact_number
* cosmetics changes (tabs and comments)
* do not send unnecessary properties once the touch is up

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@enac.fr>
Acked-by: Henrik Rydberg <rydberg@euromail.se>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>

2 files changed, 78 insertions, 59 deletions

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 9bd21482e618..97c200b0f4b8 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -292,10 +292,16 @@ config HID_MULTITOUCH
           Generic support for HID multitouch panels.
 
           Say Y here if you have one of the following devices:
-          - PixCir touchscreen
-          - Cypress TrueTouch
+          - Cypress TrueTouch panels
+          - Hanvon dual touch panels
+          - Pixcir dual touch panels
           - 'Sensing Win7-TwoFinger' panel by GeneralTouch
 
+          If unsure, say N.
+
+          To compile this driver as a module, choose M here: the
+          module will be called hid-multitouch.
+
 config HID_NTRIG
         tristate "N-Trig touch screen"
         depends on USB_HID
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 3442ed56e964..07d3183fdde5 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -32,8 +32,10 @@ MODULE_LICENSE("GPL");
 /* quirks to control the device */
 #define MT_QUIRK_NOT_SEEN_MEANS_UP      (1 << 0)
 #define MT_QUIRK_SLOT_IS_CONTACTID      (1 << 1)
-#define MT_QUIRK_CYPRESS        (1 << 2)
+#define MT_QUIRK_CYPRESS                (1 << 2)
 #define MT_QUIRK_SLOT_IS_CONTACTNUMBER  (1 << 3)
+#define MT_QUIRK_VALID_IS_INRANGE       (1 << 4)
+#define MT_QUIRK_VALID_IS_CONFIDENCE    (1 << 5)
 
 struct mt_slot {
         __s32 x, y, p, w, h;
@@ -55,6 +57,7 @@ struct mt_device {
 };
 
 struct mt_class {
+        __s32 name;     /* MT_CLS */
         __s32 quirks;
         __s32 sn_move;  /* Signal/noise ratio for move events */
         __s32 sn_pressure;      /* Signal/noise ratio for pressure events */
@@ -62,26 +65,16 @@ struct mt_class {
 };
 
 /* classes of device behavior */
-#define MT_CLS_DEFAULT 0
-#define MT_CLS_DUAL1 1
-#define MT_CLS_DUAL2 2
-#define MT_CLS_CYPRESS 3
+#define MT_CLS_DEFAULT  1
+#define MT_CLS_DUAL1    2
+#define MT_CLS_DUAL2    3
+#define MT_CLS_CYPRESS  4
 
 /*
  * these device-dependent functions determine what slot corresponds
  * to a valid contact that was just read.
  */
 
-static int slot_is_contactid(struct mt_device *td)
-{
-        return td->curdata.contactid;
-}
-
-static int slot_is_contactnumber(struct mt_device *td)
-{
-        return td->num_received;
-}
-
 static int cypress_compute_slot(struct mt_device *td)
 {
         if (td->curdata.contactid != 0 || td->num_received == 0)
@@ -103,17 +96,30 @@ static int find_slot_from_contactid(struct mt_device *td)
                         !td->slots[i].touch_state)
                         return i;
         }
-        return -1;
         /* should not occurs. If this happens that means
          * that the device sent more touches that it says
          * in the report descriptor. It is ignored then. */
+        return -1;
 }
 
 struct mt_class mt_classes[] = {
-        { 0, 0, 0, 10 },                             /* MT_CLS_DEFAULT */
-        { MT_QUIRK_SLOT_IS_CONTACTID, 0, 0, 2 },     /* MT_CLS_DUAL1 */
-        { MT_QUIRK_SLOT_IS_CONTACTNUMBER, 0, 0, 10 },    /* MT_CLS_DUAL2 */
-        { MT_QUIRK_CYPRESS | MT_QUIRK_NOT_SEEN_MEANS_UP, 0, 0, 10 }, /* MT_CLS_CYPRESS */
+        { .name = MT_CLS_DEFAULT,
+                .quirks = MT_QUIRK_VALID_IS_INRANGE,
+                .maxcontacts = 10 },
+        { .name = MT_CLS_DUAL1,
+                .quirks = MT_QUIRK_VALID_IS_INRANGE |
+                        MT_QUIRK_SLOT_IS_CONTACTID,
+                .maxcontacts = 2 },
+        { .name = MT_CLS_DUAL2,
+                .quirks = MT_QUIRK_VALID_IS_INRANGE |
+                        MT_QUIRK_SLOT_IS_CONTACTNUMBER,
+                .maxcontacts = 2 },
+        { .name = MT_CLS_CYPRESS,
+                .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP |
+                        MT_QUIRK_CYPRESS,
+                .maxcontacts = 10 },
+
+        { }
 };
 
 static void mt_feature_mapping(struct hid_device *hdev, struct hid_input *hi,
@@ -192,7 +198,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
                         hid_map_usage(hi, usage, bit, max,
                                         EV_ABS, ABS_MT_TOUCH_MINOR);
                         field->logical_maximum = 1;
-                        field->logical_minimum = 1;
+                        field->logical_minimum = 0;
                         set_abs(hi->input, ABS_MT_ORIENTATION, field, 0);
                         td->last_slot_field = usage->hid;
                         return 1;
@@ -237,16 +243,16 @@ static int mt_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 
 static int mt_compute_slot(struct mt_device *td)
 {
-        struct mt_class *cls = td->mtclass;
+        __s32 quirks = td->mtclass->quirks;
 
-        if (cls->quirks & MT_QUIRK_SLOT_IS_CONTACTID)
-                return slot_is_contactid(td);
+        if (quirks & MT_QUIRK_SLOT_IS_CONTACTID)
+                return td->curdata.contactid;
 
-        if (cls->quirks & MT_QUIRK_CYPRESS)
+        if (quirks & MT_QUIRK_CYPRESS)
                 return cypress_compute_slot(td);
 
-        if (cls->quirks & MT_QUIRK_SLOT_IS_CONTACTNUMBER)
-                return slot_is_contactnumber(td);
+        if (quirks & MT_QUIRK_SLOT_IS_CONTACTNUMBER)
+                return td->num_received;
 
         return find_slot_from_contactid(td);
 }
@@ -257,16 +263,12 @@ static int mt_compute_slot(struct mt_device *td)
  */
 static void mt_complete_slot(struct mt_device *td)
 {
+        td->curdata.seen_in_this_frame = true;
         if (td->curvalid) {
-                struct mt_slot *slot;
                 int slotnum = mt_compute_slot(td);
 
-                if (slotnum >= 0 && slotnum < td->mtclass->maxcontacts) {
-                        slot = td->slots + slotnum;
-
-                        memcpy(slot, &(td->curdata), sizeof(struct mt_slot));
-                        slot->seen_in_this_frame = true;
-                }
+                if (slotnum >= 0 && slotnum < td->mtclass->maxcontacts)
+                        td->slots[slotnum] = td->curdata;
         }
         td->num_received++;
 }
@@ -284,21 +286,19 @@ static void mt_emit_event(struct mt_device *td, struct input_dev *input)
                 struct mt_slot *s = &(td->slots[i]);
                 if ((td->mtclass->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP) &&
                         !s->seen_in_this_frame) {
-                        /*
-                         * this slot does not contain useful data,
-                         * notify its closure
-                         */
                         s->touch_state = false;
                 }
 
                 input_mt_slot(input, i);
                 input_mt_report_slot_state(input, MT_TOOL_FINGER,
                         s->touch_state);
-                input_event(input, EV_ABS, ABS_MT_POSITION_X, s->x);
-                input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y);
-                input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
-                input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, s->w);
-                input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, s->h);
+                if (s->touch_state) {
+                        input_event(input, EV_ABS, ABS_MT_POSITION_X, s->x);
+                        input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y);
+                        input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
+                        input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, s->w);
+                        input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, s->h);
+                }
                 s->seen_in_this_frame = false;
 
         }
@@ -314,16 +314,22 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
                                 struct hid_usage *usage, __s32 value)
 {
         struct mt_device *td = hid_get_drvdata(hid);
+        __s32 quirks = td->mtclass->quirks;
 
         if (hid->claimed & HID_CLAIMED_INPUT) {
                 switch (usage->hid) {
                 case HID_DG_INRANGE:
+                        if (quirks & MT_QUIRK_VALID_IS_INRANGE)
+                                td->curvalid = value;
                         break;
                 case HID_DG_TIPSWITCH:
-                        td->curvalid = value;
+                        if (quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
+                                td->curvalid = value;
                         td->curdata.touch_state = value;
                         break;
                 case HID_DG_CONFIDENCE:
+                        if (quirks & MT_QUIRK_VALID_IS_CONFIDENCE)
+                                td->curvalid = value;
                         break;
                 case HID_DG_CONTACTID:
                         td->curdata.contactid = value;
@@ -345,26 +351,26 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
                         break;
                 case HID_DG_CONTACTCOUNT:
                         /*
-                         * We must not overwrite the previous value (some
-                         * devices send one sequence splitted over several
-                         * messages)
+                         * Includes multi-packet support where subsequent
+                         * packets are sent with zero contactcount.
                          */
                         if (value)
-                                td->num_expected = value - 1;
+                                td->num_expected = value;
                         break;
 
                 default:
                         /* fallback to the generic hidinput handling */
                         return 0;
                 }
-        }
 
-        if (usage->hid == td->last_slot_field)
-                mt_complete_slot(td);
+                if (usage->hid == td->last_slot_field)
+                        mt_complete_slot(td);
+
+                if (field->index == td->last_field_index
+                        && td->num_received >= td->num_expected)
+                        mt_emit_event(td, field->hidinput->input);
 
-        if (field->index == td->last_field_index
-                && td->num_received > td->num_expected)
-                mt_emit_event(td, field->hidinput->input);
+        }
 
         /* we have handled the hidinput part, now remains hiddev */
         if (hid->claimed & HID_CLAIMED_HIDDEV && hid->hiddev_hid_event)
@@ -392,9 +398,16 @@ static void mt_set_input_mode(struct hid_device *hdev)
 
 static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
-        int ret;
+        int ret, i;
         struct mt_device *td;
-        struct mt_class *mtclass = mt_classes + id->driver_data;
+        struct mt_class *mtclass = mt_classes; /* MT_CLS_DEFAULT */
+
+        for (i = 0; mt_classes[i].name ; i++) {
+                if (id->driver_data == mt_classes[i].name) {
+                        mtclass = &(mt_classes[i]);
+                        break;
+                }
+        }
 
         /* This allows the driver to correctly support devices
          * that emit events over several HID messages.
@@ -417,7 +430,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
                 goto fail;
 
         ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
-        if (ret != 0)
+        if (ret)
                 goto fail;
 
         mt_set_input_mode(hdev);