[PATCH v2 0/3] Support of Perixx Peripad 701 for hid-multitouch

[PATCH v2 0/3] Support of Perixx Peripad 701 for hid-multitouch

[Benjamin Tissoires]

Hi Guys,

This is the v2 of the support of Perixx Peripad 701.

I included the changes asked by Henrik:
- refactor:
+             code = ((usage->hid - 1) & HID_USAGE);
+             code += BTN_MOUSE;
into:
+             code = ((usage->hid - 1) & HID_USAGE) + BTN_MOUSE;

- renamed field maxcontactnumber into maxcontact_report_id
- split the support of the device and the control of the feature Max Contact Number.
- fix typo in commit message

Cheers,
Benjamin

[PATCH v2 1/3] hid-multitouch: add support for trackpads

[Benjamin Tissoires]

* some multitouch trackpads present the touch usage. This needs to be
filtered as it will conflict with mt-implementation.
* trackpads send BTN_TOOL_* to notify how many fingers are present
(this is used by xorg to use synaptics instead of generic evdev)
* trackpads like Perixx 701 are not different from a hid point of view
from a touchscreen, and we need to manually set them as touchpad.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@enac.fr>
---
 drivers/hid/hid-multitouch.c |   43 ++++++++++++++++++++++++++++++++++++-----
 1 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 24fc442..ef59140 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1,9 +1,9 @@
 /*
  *  HID driver for multitouch panels
  *
- *  Copyright (c) 2010-2011 Stephane Chatty <chatty@enac.fr>
- *  Copyright (c) 2010-2011 Benjamin Tissoires <benjamin.tissoires@gmail.com>
- *  Copyright (c) 2010-2011 Ecole Nationale de l'Aviation Civile, France
+ *  Copyright (c) 2010-2012 Stephane Chatty <chatty@enac.fr>
+ *  Copyright (c) 2010-2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
+ *  Copyright (c) 2010-2012 Ecole Nationale de l'Aviation Civile, France
  *
  *  This code is partly based on hid-egalax.c:
  *
@@ -67,6 +67,7 @@ struct mt_class {
 	__s32 sn_height;	/* Signal/noise ratio for height events */
 	__s32 sn_pressure;	/* Signal/noise ratio for pressure events */
 	__u8 maxcontacts;
+	bool is_indirect;	/* true for touchpads */
 };
 
 struct mt_device {
@@ -265,17 +266,31 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 {
 	struct mt_device *td = hid_get_drvdata(hdev);
 	struct mt_class *cls = &td->mtclass;
+	int code;
 
 	/* 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
+	else if (field->application != HID_DG_TOUCHPAD)
 		return 0;
 
+	/* In case of an indirect device (touchpad), we need to add
+	 * specific BTN_TOOL_* to be handled by the synaptics xorg
+	 * driver.
+	 * We also consider that touchscreens providing buttons are touchpads.
+	 */
+	if (field->application == HID_DG_TOUCHPAD ||
+	    (usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON ||
+	    cls->is_indirect) {
+		set_bit(INPUT_PROP_POINTER, hi->input->propbit);
+		set_bit(BTN_TOOL_FINGER, hi->input->keybit);
+		set_bit(BTN_TOOL_DOUBLETAP, hi->input->keybit);
+		set_bit(BTN_TOOL_TRIPLETAP, hi->input->keybit);
+		set_bit(BTN_TOOL_QUADTAP, hi->input->keybit);
+	}
+
 	/* eGalax devices provide a Digitizer.Stylus input which overrides
 	 * the correct Digitizers.Finger X/Y ranges.
 	 * Let's just ignore this input. */
@@ -389,9 +404,19 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 				td->last_field_index = field->index;
 			return -1;
 		}
+		case HID_DG_TOUCH:
+			/* Legacy devices use TIPSWITCH and not TOUCH.
+			 * Let's just ignore this field. */
+			return -1;
 		/* let hid-input decide for the others */
 		return 0;
 
+	case HID_UP_BUTTON:
+		code = ((usage->hid - 1) & HID_USAGE) + BTN_MOUSE;
+		hid_map_usage(hi, usage, bit, max, EV_KEY, code);
+		input_set_capability(hi->input, EV_KEY, code);
+		return 1;
+
 	case 0xff000000:
 		/* we do not want to map these: no input-oriented meaning */
 		return -1;
@@ -453,6 +478,7 @@ static void mt_complete_slot(struct mt_device *td)
 static void mt_emit_event(struct mt_device *td, struct input_dev *input)
 {
 	int i;
+	int finger_count = 0;
 
 	for (i = 0; i < td->maxcontacts; ++i) {
 		struct mt_slot *s = &(td->slots[i]);
@@ -477,12 +503,14 @@ static void mt_emit_event(struct mt_device *td, struct input_dev *input)
 			input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
 			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
 			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
+			finger_count++;
 		}
 		s->seen_in_this_frame = false;
 
 	}
 
 	input_mt_report_pointer_emulation(input, true);
+	input_mt_report_finger_count(input, finger_count);
 	input_sync(input);
 	td->num_received = 0;
 }
@@ -538,6 +566,9 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
 			if (value)
 				td->num_expected = value;
 			break;
+		case HID_DG_TOUCH:
+			/* do nothing */
+			break;
 
 		default:
 			/* fallback to the generic hidinput handling */
-- 
1.7.4.4

Re: [PATCH v2 1/3] hid-multitouch: add support for trackpads

[Henrik Rydberg]

Hi Benjamin,

> @@ -389,9 +404,19 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  				td->last_field_index = field->index;
>  			return -1;
>  		}
> +		case HID_DG_TOUCH:
> +			/* Legacy devices use TIPSWITCH and not TOUCH.
> +			 * Let's just ignore this field. */
> +			return -1;
>  		/* let hid-input decide for the others */
>  		return 0;
>  
> +	case HID_UP_BUTTON:
> +		code = ((usage->hid - 1) & HID_USAGE) + BTN_MOUSE;

Why '+' here instead of '|' is  beyond me...

> +		hid_map_usage(hi, usage, bit, max, EV_KEY, code);
> +		input_set_capability(hi->input, EV_KEY, code);
> +		return 1;
> +
>  	case 0xff000000:
>  		/* we do not want to map these: no input-oriented meaning */
>  		return -1;
> @@ -453,6 +478,7 @@ static void mt_complete_slot(struct mt_device *td)
>  static void mt_emit_event(struct mt_device *td, struct input_dev *input)
>  {
>  	int i;
> +	int finger_count = 0;
>  
>  	for (i = 0; i < td->maxcontacts; ++i) {
>  		struct mt_slot *s = &(td->slots[i]);
> @@ -477,12 +503,14 @@ static void mt_emit_event(struct mt_device *td, struct input_dev *input)
>  			input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
>  			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
>  			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
> +			finger_count++;
>  		}
>  		s->seen_in_this_frame = false;
>  
>  	}
>  
>  	input_mt_report_pointer_emulation(input, true);
> +	input_mt_report_finger_count(input, finger_count);

This function is already called from input_mt_report_pointer_emulation().

>  	input_sync(input);
>  	td->num_received = 0;
>  }

Thanks,
Henrik

Re: [PATCH v2 1/3] hid-multitouch: add support for trackpads

[Dmitry Torokhov]

On Fri, Jan 20, 2012 at 05:09:48PM +0100, Henrik Rydberg wrote:
> Hi Benjamin,
> 
> > @@ -389,9 +404,19 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> >  				td->last_field_index = field->index;
> >  			return -1;
> >  		}
> > +		case HID_DG_TOUCH:
> > +			/* Legacy devices use TIPSWITCH and not TOUCH.
> > +			 * Let's just ignore this field. */
> > +			return -1;
> >  		/* let hid-input decide for the others */
> >  		return 0;
> >  
> > +	case HID_UP_BUTTON:
> > +		code = ((usage->hid - 1) & HID_USAGE) + BTN_MOUSE;
> 
> Why '+' here instead of '|' is  beyond me...

Because it is an offset for the range. The fact that it is on power 2
boundary and we can use "|" is purely coincidential here.

Maybe we should even write it as:

		code = BTN_MOUSE + ((usage->hid - 1) & HID_USAGE);

Thanks,
Dmitry

-- 
Dmitry

Re: [PATCH v2 1/3] hid-multitouch: add support for trackpads

[Benjamin Tissoires]

Hi Dmitry and Henrik,

Thanks Dmitry for the comment.

I'll do the change as you mentioned and will also remove the
input_mt_report_finger_count call.

Cheers,
Benjamin

On Fri, Jan 20, 2012 at 17:48, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Jan 20, 2012 at 05:09:48PM +0100, Henrik Rydberg wrote:
>> Hi Benjamin,
>>
>> > @@ -389,9 +404,19 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> >                             td->last_field_index = field->index;
>> >                     return -1;
>> >             }
>> > +           case HID_DG_TOUCH:
>> > +                   /* Legacy devices use TIPSWITCH and not TOUCH.
>> > +                    * Let's just ignore this field. */
>> > +                   return -1;
>> >             /* let hid-input decide for the others */
>> >             return 0;
>> >
>> > +   case HID_UP_BUTTON:
>> > +           code = ((usage->hid - 1) & HID_USAGE) + BTN_MOUSE;
>>
>> Why '+' here instead of '|' is  beyond me...
>
> Because it is an offset for the range. The fact that it is on power 2
> boundary and we can use "|" is purely coincidential here.
>
> Maybe we should even write it as:
>
>                code = BTN_MOUSE + ((usage->hid - 1) & HID_USAGE);
>
> Thanks,
> Dmitry
>
> --
> Dmitry
> --
> 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
--
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

Re: [PATCH v2 1/3] hid-multitouch: add support for trackpads

[Benjamin Tissoires]

Hi Dmitry and Henrik,

Thanks Dmitry for the comment.

I'll do the change as you mentioned and will also remove the
input_mt_report_finger_count call.

Cheers,
Benjamin

On Fri, Jan 20, 2012 at 17:48, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Jan 20, 2012 at 05:09:48PM +0100, Henrik Rydberg wrote:
>> Hi Benjamin,
>>
>> > @@ -389,9 +404,19 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> >                             td->last_field_index = field->index;
>> >                     return -1;
>> >             }
>> > +           case HID_DG_TOUCH:
>> > +                   /* Legacy devices use TIPSWITCH and not TOUCH.
>> > +                    * Let's just ignore this field. */
>> > +                   return -1;
>> >             /* let hid-input decide for the others */
>> >             return 0;
>> >
>> > +   case HID_UP_BUTTON:
>> > +           code = ((usage->hid - 1) & HID_USAGE) + BTN_MOUSE;
>>
>> Why '+' here instead of '|' is  beyond me...
>
> Because it is an offset for the range. The fact that it is on power 2
> boundary and we can use "|" is purely coincidential here.
>
> Maybe we should even write it as:
>
>                code = BTN_MOUSE + ((usage->hid - 1) & HID_USAGE);
>
> Thanks,
> Dmitry
>
> --
> Dmitry
> --
> 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

Re: [PATCH v2 1/3] hid-multitouch: add support for trackpads

[Jiri Kosina]

On Tue, 24 Jan 2012, Benjamin Tissoires wrote:

> Hi Dmitry and Henrik,
> 
> Thanks Dmitry for the comment.
> 
> I'll do the change as you mentioned and will also remove the
> input_mt_report_finger_count call.

Benjamin,

I have just materialized from almost 3 weeks of being offline, so I just 
wanted to check the current status of this.

Are you planning to send v3 of this patchset?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2 1/3] hid-multitouch: add support for trackpads

[Benjamin Tissoires]

Hi Jiri,

On Thu, Feb 2, 2012 at 09:36, Jiri Kosina <jkosina@suse.cz> wrote:
> On Tue, 24 Jan 2012, Benjamin Tissoires wrote:
>
>> Hi Dmitry and Henrik,
>>
>> Thanks Dmitry for the comment.
>>
>> I'll do the change as you mentioned and will also remove the
>> input_mt_report_finger_count call.
>
> Benjamin,
>
> I have just materialized from almost 3 weeks of being offline, so I just
> wanted to check the current status of this.

no problems on my side (was quite busy)

>
> Are you planning to send v3 of this patchset?

sure. I'll try to do it today.

Cheers,
Benjamin

>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
> --
> 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
--
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

Re: [PATCH v2 1/3] hid-multitouch: add support for trackpads

[Benjamin Tissoires]

Hi Jiri,

On Thu, Feb 2, 2012 at 09:36, Jiri Kosina <jkosina@suse.cz> wrote:
> On Tue, 24 Jan 2012, Benjamin Tissoires wrote:
>
>> Hi Dmitry and Henrik,
>>
>> Thanks Dmitry for the comment.
>>
>> I'll do the change as you mentioned and will also remove the
>> input_mt_report_finger_count call.
>
> Benjamin,
>
> I have just materialized from almost 3 weeks of being offline, so I just
> wanted to check the current status of this.

no problems on my side (was quite busy)

>
> Are you planning to send v3 of this patchset?

sure. I'll try to do it today.

Cheers,
Benjamin

>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
> --
> 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

[PATCH v2 2/3] HID: multitouch: add control of the feature "Maximum Contact Number"

[Benjamin Tissoires]

Some devices, like Perixx Peripad 701 do not work if the feature
"Maximum Contact Number" is not set to the right value.
This patch allows hid-multitouch to control this feature.

If the programmer fills the field maxcontacts in the mt_class,
then the driver will set the feature to this value. It is safe
for current drivers as the feature is read/right in the HID norm
and all devices should implement the norm.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@enac.fr>
---
 drivers/hid/hid-multitouch.c |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index ef59140..fd978c9 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -77,6 +77,8 @@ struct mt_device {
 	unsigned last_slot_field;	/* the last field of a slot */
 	int last_mt_collection;	/* last known mt-related collection */
 	__s8 inputmode;		/* InputMode HID feature, -1 if non-existent */
+	__s8 maxcontact_report_id;	/* Maximum Contact Number HID feature,
+				   -1 if non-existent */
 	__u8 num_received;	/* how many contacts we received */
 	__u8 num_expected;	/* expected last contact index */
 	__u8 maxcontacts;
@@ -242,6 +244,7 @@ static void mt_feature_mapping(struct hid_device *hdev,
 		td->inputmode = field->report->id;
 		break;
 	case HID_DG_CONTACTMAX:
+		td->maxcontact_report_id = field->report->id;
 		td->maxcontacts = field->value[0];
 		if (td->mtclass.maxcontacts)
 			/* check if the maxcontacts is given by the class */
@@ -609,6 +612,36 @@ static void mt_set_input_mode(struct hid_device *hdev)
 	}
 }
 
+static void mt_set_maxcontacts(struct hid_device *hdev)
+{
+	struct mt_device *td = hid_get_drvdata(hdev);
+	struct hid_report *r;
+	struct hid_report_enum *re;
+	int fieldmax, max;
+
+	if (td->maxcontact_report_id < 0)
+		return;
+
+	if (!td->mtclass.maxcontacts)
+		return;
+
+	re = &(hdev->report_enum[HID_FEATURE_REPORT]);
+	r = re->report_id_hash[td->maxcontact_report_id];
+	if (r) {
+		max = td->mtclass.maxcontacts;
+		fieldmax = r->field[0]->logical_maximum;
+		hid_info(hdev, "%s: value = %d / %d / %d\n", __func__,
+			r->field[0]->value[0],
+			td->mtclass.maxcontacts,
+			fieldmax);
+		max = fieldmax < max ? fieldmax : max;
+		if (r->field[0]->value[0] != max) {
+			r->field[0]->value[0] = max;
+			usbhid_submit_report(hdev, r, USB_DIR_OUT);
+		}
+	}
+}
+
 static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	int ret, i;
@@ -634,6 +667,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 	td->mtclass = *mtclass;
 	td->inputmode = -1;
+	td->maxcontact_report_id = -1;
 	td->last_mt_collection = -1;
 	hid_set_drvdata(hdev, td);
 
@@ -656,6 +690,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
 
+	mt_set_maxcontacts(hdev);
 	mt_set_input_mode(hdev);
 
 	return 0;
@@ -668,6 +703,7 @@ fail:
 #ifdef CONFIG_PM
 static int mt_reset_resume(struct hid_device *hdev)
 {
+	mt_set_maxcontacts(hdev);
 	mt_set_input_mode(hdev);
 	return 0;
 }
-- 
1.7.4.4

Re: [PATCH v2 2/3] HID: multitouch: add control of the feature "Maximum Contact Number"

[Henrik Rydberg]

Hi Benjamin,

> If the programmer fills the field maxcontacts in the mt_class,
> then the driver will set the feature to this value. It is safe
> for current drivers as the feature is read/right in the HID norm

read/write, I presume.

> @@ -609,6 +612,36 @@ static void mt_set_input_mode(struct hid_device *hdev)
>  	}
>  }
>  
> +static void mt_set_maxcontacts(struct hid_device *hdev)
> +{
> +	struct mt_device *td = hid_get_drvdata(hdev);
> +	struct hid_report *r;
> +	struct hid_report_enum *re;
> +	int fieldmax, max;
> +
> +	if (td->maxcontact_report_id < 0)
> +		return;
> +
> +	if (!td->mtclass.maxcontacts)
> +		return;
> +
> +	re = &(hdev->report_enum[HID_FEATURE_REPORT]);

Why parenthesis here?

> +	r = re->report_id_hash[td->maxcontact_report_id];
> +	if (r) {
> +		max = td->mtclass.maxcontacts;
> +		fieldmax = r->field[0]->logical_maximum;
> +		hid_info(hdev, "%s: value = %d / %d / %d\n", __func__,
> +			r->field[0]->value[0],
> +			td->mtclass.maxcontacts,
> +			fieldmax);

Looks like debug information, please downplay or remove.

> +		max = fieldmax < max ? fieldmax : max;

There are kernel variants of min/max functions to consider here.

> +		if (r->field[0]->value[0] != max) {
> +			r->field[0]->value[0] = max;
> +			usbhid_submit_report(hdev, r, USB_DIR_OUT);
> +		}

So the report is in fact only set when different... Do we know for
which current devices this is true?

Thanks,
Henrik

Re: [PATCH v2 2/3] HID: multitouch: add control of the feature "Maximum Contact Number"

[Benjamin Tissoires]

Hi Henrik,

On Fri, Jan 20, 2012 at 17:19, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Benjamin,
>
>> If the programmer fills the field maxcontacts in the mt_class,
>> then the driver will set the feature to this value. It is safe
>> for current drivers as the feature is read/right in the HID norm
>
> read/write, I presume.

you're right... ;-)

>
>> @@ -609,6 +612,36 @@ static void mt_set_input_mode(struct hid_device *hdev)
>>       }
>>  }
>>
>> +static void mt_set_maxcontacts(struct hid_device *hdev)
>> +{
>> +     struct mt_device *td = hid_get_drvdata(hdev);
>> +     struct hid_report *r;
>> +     struct hid_report_enum *re;
>> +     int fieldmax, max;
>> +
>> +     if (td->maxcontact_report_id < 0)
>> +             return;
>> +
>> +     if (!td->mtclass.maxcontacts)
>> +             return;
>> +
>> +     re = &(hdev->report_enum[HID_FEATURE_REPORT]);
>
> Why parenthesis here?

ok

>
>> +     r = re->report_id_hash[td->maxcontact_report_id];
>> +     if (r) {
>> +             max = td->mtclass.maxcontacts;
>> +             fieldmax = r->field[0]->logical_maximum;
>> +             hid_info(hdev, "%s: value = %d / %d / %d\n", __func__,
>> +                     r->field[0]->value[0],
>> +                     td->mtclass.maxcontacts,
>> +                     fieldmax);
>
> Looks like debug information, please downplay or remove.

I can remove them if you need.

>
>> +             max = fieldmax < max ? fieldmax : max;
>
> There are kernel variants of min/max functions to consider here.

I'll look for them.

>
>> +             if (r->field[0]->value[0] != max) {
>> +                     r->field[0]->value[0] = max;
>> +                     usbhid_submit_report(hdev, r, USB_DIR_OUT);
>> +             }
>
> So the report is in fact only set when different... Do we know for
> which current devices this is true?

Currently, I've got only the Perixx touchpad that needs this feature
to be set. The initial value is 0 whereas it should be 2.

Maybe some drivers that used the DUAL_MT_CLS_* can handle more, but I
have no idea has I don't have many of them.

Cheers,
Benjamin

>
> Thanks,
> Henrik
--
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

Re: [PATCH v2 2/3] HID: multitouch: add control of the feature "Maximum Contact Number"

[Benjamin Tissoires]

Hi Henrik,

On Fri, Jan 20, 2012 at 17:19, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Benjamin,
>
>> If the programmer fills the field maxcontacts in the mt_class,
>> then the driver will set the feature to this value. It is safe
>> for current drivers as the feature is read/right in the HID norm
>
> read/write, I presume.

you're right... ;-)

>
>> @@ -609,6 +612,36 @@ static void mt_set_input_mode(struct hid_device *hdev)
>>       }
>>  }
>>
>> +static void mt_set_maxcontacts(struct hid_device *hdev)
>> +{
>> +     struct mt_device *td = hid_get_drvdata(hdev);
>> +     struct hid_report *r;
>> +     struct hid_report_enum *re;
>> +     int fieldmax, max;
>> +
>> +     if (td->maxcontact_report_id < 0)
>> +             return;
>> +
>> +     if (!td->mtclass.maxcontacts)
>> +             return;
>> +
>> +     re = &(hdev->report_enum[HID_FEATURE_REPORT]);
>
> Why parenthesis here?

ok

>
>> +     r = re->report_id_hash[td->maxcontact_report_id];
>> +     if (r) {
>> +             max = td->mtclass.maxcontacts;
>> +             fieldmax = r->field[0]->logical_maximum;
>> +             hid_info(hdev, "%s: value = %d / %d / %d\n", __func__,
>> +                     r->field[0]->value[0],
>> +                     td->mtclass.maxcontacts,
>> +                     fieldmax);
>
> Looks like debug information, please downplay or remove.

I can remove them if you need.

>
>> +             max = fieldmax < max ? fieldmax : max;
>
> There are kernel variants of min/max functions to consider here.

I'll look for them.

>
>> +             if (r->field[0]->value[0] != max) {
>> +                     r->field[0]->value[0] = max;
>> +                     usbhid_submit_report(hdev, r, USB_DIR_OUT);
>> +             }
>
> So the report is in fact only set when different... Do we know for
> which current devices this is true?

Currently, I've got only the Perixx touchpad that needs this feature
to be set. The initial value is 0 whereas it should be 2.

Maybe some drivers that used the DUAL_MT_CLS_* can handle more, but I
have no idea has I don't have many of them.

Cheers,
Benjamin

>
> Thanks,
> Henrik

[PATCH v2 3/3] HID: multitouch: support Perixx PERIPAD 701

[Benjamin Tissoires]

Perixx Peripad 701 is an hybrid device which presents a touchpad and
a keyboard on the same surface. The switch between the two is controlled
by a physical switch, and the firmware sends the events on the right
interface (mouse, keyboard or multitouch).
This patch enables the multitouch interface of this device to work.

We need to manually declare the device as a trackpad (we cannot infer it
from the reports descriptors as the device works under Windows, a system
that does not allow multitouch touchpad).
We also need to set the hid feature MAX CONTACT NUMBER to 2 or the device
stops sending events once it has been pressed by two touches.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@enac.fr>
---
 drivers/hid/Kconfig          |    1 +
 drivers/hid/hid-ids.h        |    1 +
 drivers/hid/hid-multitouch.c |   11 +++++++++++
 3 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index a421abd..f7c43b6 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -355,6 +355,7 @@ config HID_MULTITOUCH
 	  - Lumio CrystalTouch panels
 	  - MosArt dual-touch panels
 	  - PenMount dual touch panels
+	  - Perixx Peripad 701 touchpad
 	  - PixArt optical touch screen
 	  - Pixcir dual touch panels
 	  - Quanta panels
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index b8574cd..662a0b6 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -659,6 +659,7 @@
 
 #define USB_VENDOR_ID_TOPSEED2		0x1784
 #define USB_DEVICE_ID_TOPSEED2_RF_COMBO	0x0004
+#define USB_DEVICE_ID_TOPSEED2_PERIPAD_701	0x0016
 
 #define USB_VENDOR_ID_TOPMAX		0x0663
 #define USB_DEVICE_ID_TOPMAX_COBRAPAD	0x0103
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index fd978c9..8a6b823 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -103,6 +103,7 @@ struct mt_device {
 #define MT_CLS_CYPRESS				0x0102
 #define MT_CLS_EGALAX				0x0103
 #define MT_CLS_EGALAX_SERIAL			0x0104
+#define MT_CLS_TOPSEED				0x0105
 
 #define MT_DEFAULT_MAXCONTACT	10
 
@@ -192,6 +193,11 @@ static struct mt_class mt_classes[] = {
 		.sn_move = 4096,
 		.sn_pressure = 32,
 	},
+	{ .name = MT_CLS_TOPSEED,
+		.quirks = MT_QUIRK_ALWAYS_VALID,
+		.is_indirect = true,
+		.maxcontacts = 2,
+	},
 
 	{ }
 };
@@ -904,6 +910,11 @@ static const struct hid_device_id mt_devices[] = {
 		HID_USB_DEVICE(USB_VENDOR_ID_STANTUM_SITRONIX,
 			USB_DEVICE_ID_MTP_SITRONIX)},
 
+	/* TopSeed panels */
+	{ .driver_data = MT_CLS_TOPSEED,
+		HID_USB_DEVICE(USB_VENDOR_ID_TOPSEED2,
+			USB_DEVICE_ID_TOPSEED2_PERIPAD_701) },
+
 	/* Touch International panels */
 	{ .driver_data = MT_CLS_DEFAULT,
 		HID_USB_DEVICE(USB_VENDOR_ID_TOUCH_INTL,
-- 
1.7.4.4