[PATCH v4 1/4] HID: multitouch: Fix alphabetic sorting of mt_devices table.

[PATCH v4 1/4] HID: multitouch: Fix alphabetic sorting of mt_devices table.

[Hans de Goede]

Fix alphabetic sorting of mt_devices hid_device_id table.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-multitouch.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 9e8c4d2ba11d..bb939f6990f1 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1605,14 +1605,6 @@ static const struct hid_device_id mt_devices[] = {
 		MT_USB_DEVICE(USB_VENDOR_ID_TURBOX,
 			USB_DEVICE_ID_TURBOX_TOUCHSCREEN_MOSART) },
 
-	/* Panasonic panels */
-	{ .driver_data = MT_CLS_PANASONIC,
-		MT_USB_DEVICE(USB_VENDOR_ID_PANASONIC,
-			USB_DEVICE_ID_PANABOARD_UBT780) },
-	{ .driver_data = MT_CLS_PANASONIC,
-		MT_USB_DEVICE(USB_VENDOR_ID_PANASONIC,
-			USB_DEVICE_ID_PANABOARD_UBT880) },
-
 	/* Novatek Panel */
 	{ .driver_data = MT_CLS_NSMU,
 		MT_USB_DEVICE(USB_VENDOR_ID_NOVATEK,
@@ -1623,6 +1615,14 @@ static const struct hid_device_id mt_devices[] = {
 		HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
 			USB_VENDOR_ID_NTRIG, 0x1b05) },
 
+	/* Panasonic panels */
+	{ .driver_data = MT_CLS_PANASONIC,
+		MT_USB_DEVICE(USB_VENDOR_ID_PANASONIC,
+			USB_DEVICE_ID_PANABOARD_UBT780) },
+	{ .driver_data = MT_CLS_PANASONIC,
+		MT_USB_DEVICE(USB_VENDOR_ID_PANASONIC,
+			USB_DEVICE_ID_PANABOARD_UBT880) },
+
 	/* PixArt optical touch screen */
 	{ .driver_data = MT_CLS_INRANGE_CONTACTNUMBER,
 		MT_USB_DEVICE(USB_VENDOR_ID_PIXART,
-- 
2.14.3

[PATCH v4 2/4] HID: multitouch: Properly deal with Win8 PTP reports with 0 touches

[Hans de Goede]

The Windows Precision Touchpad spec "Figure 4 Button Only Down and Up"
and "Table 9 Report Sequence for Button Only Down and Up" indicate
that the first packet of a (possibly hybrid mode multi-packet) frame
may contain a contact-count of 0 if only a button is pressed and no
fingers are detected.

This means that a value of 0 for contact-count is a valid value and
should be used as expected contact count when it is the first packet
(num_received == 0), as extra check to make sure that this is the first
packet of a buttons only frame, we also check that the timestamp is
different.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
Changes in v2:
-New patch in v2 of this patch-set

Changes in v3:
-Also check timestamps to make sure we're dealing with the first-packet
 of a frame

Changes in v4:
-Add Benjamin's Reviewed-by
---
 drivers/hid/hid-multitouch.c | 34 ++++++++++++++++++++++++++++++++--
 include/linux/hid.h          |  1 +
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index bb939f6990f1..5d3904d0e89d 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -117,6 +117,9 @@ struct mt_device {
 	unsigned long mt_io_flags;	/* mt flags (MT_IO_FLAGS_*) */
 	int cc_index;	/* contact count field index in the report */
 	int cc_value_index;	/* contact count value index in the field */
+	int scantime_index;	/* scantime field index in the report */
+	int scantime_val_index;	/* scantime value index in the field */
+	int prev_scantime;	/* scantime reported in the previous packet */
 	unsigned last_slot_field;	/* the last field of a slot */
 	unsigned mt_report_id;	/* the report ID of the multitouch device */
 	unsigned long initial_quirks;	/* initial quirks state */
@@ -595,6 +598,14 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			/* we don't set td->last_slot_field as contactcount and
 			 * contact max are global to the report */
 			return -1;
+		case HID_DG_SCANTIME:
+			/* Ignore if indexes are out of bounds. */
+			if (field->index >= field->report->maxfield ||
+			    usage->usage_index >= field->report_count)
+				return 1;
+			td->scantime_index = field->index;
+			td->scantime_val_index = usage->usage_index;
+			return 1;
 		case HID_DG_TOUCH:
 			/* Legacy devices use TIPSWITCH and not TOUCH.
 			 * Let's just ignore this field. */
@@ -812,9 +823,10 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
 static void mt_touch_report(struct hid_device *hid, struct hid_report *report)
 {
 	struct mt_device *td = hid_get_drvdata(hid);
+	__s32 cls = td->mtclass.name;
 	struct hid_field *field;
 	unsigned count;
-	int r, n;
+	int r, n, scantime = 0;
 
 	/* sticky fingers release in progress, abort */
 	if (test_and_set_bit(MT_IO_FLAGS_RUNNING, &td->mt_io_flags))
@@ -824,12 +836,29 @@ static void mt_touch_report(struct hid_device *hid, struct hid_report *report)
 	 * Includes multi-packet support where subsequent
 	 * packets are sent with zero contactcount.
 	 */
+	if (td->scantime_index >= 0) {
+		field = report->field[td->scantime_index];
+		scantime = field->value[td->scantime_val_index];
+	}
 	if (td->cc_index >= 0) {
 		struct hid_field *field = report->field[td->cc_index];
 		int value = field->value[td->cc_value_index];
-		if (value)
+
+		/*
+		 * For Win8 PTPs the first packet (td->num_received == 0) may
+		 * have a contactcount of 0 if there only is a button event.
+		 * We double check that this is not a continuation packet
+		 * of a possible multi-packet frame be checking that the
+		 * timestamp has changed.
+		 */
+		if ((cls == MT_CLS_WIN_8 || cls == MT_CLS_WIN_8_DUAL) &&
+		    td->num_received == 0 && td->prev_scantime != scantime)
+			td->num_expected = value;
+		/* A non 0 contact count always indicates a first packet */
+		else if (value)
 			td->num_expected = value;
 	}
+	td->prev_scantime = scantime;
 
 	for (r = 0; r < report->maxfield; r++) {
 		field = report->field[r];
@@ -1285,6 +1314,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	td->maxcontact_report_id = -1;
 	td->inputmode_value = MT_INPUTMODE_TOUCHSCREEN;
 	td->cc_index = -1;
+	td->scantime_index = -1;
 	td->mt_report_id = -1;
 	hid_set_drvdata(hdev, td);
 
diff --git a/include/linux/hid.h b/include/linux/hid.h
index ab05a86269dc..47dd962d9a7a 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -289,6 +289,7 @@ struct hid_item {
 #define HID_DG_DEVICEINDEX	0x000d0053
 #define HID_DG_CONTACTCOUNT	0x000d0054
 #define HID_DG_CONTACTMAX	0x000d0055
+#define HID_DG_SCANTIME		0x000d0056
 #define HID_DG_BUTTONTYPE	0x000d0059
 #define HID_DG_BARRELSWITCH2	0x000d005a
 #define HID_DG_TOOLSERIALNUMBER	0x000d005b
-- 
2.14.3

[PATCH v4 3/4] HID: multitouch: Only look at non touch fields in first packet of a frame

[Hans de Goede]

Devices in "single finger hybrid mode" will send one report per finger,
on some devices only the first report of such a multi-packet frame will
contain a value for BTN_LEFT, in subsequent reports (if multiple fingers
are down) the value is always 0, causing hid-mt to report BTN_LEFT going
1 - 0 - 1 - 0 when pressing a clickpad and putting down a second finger.
This happens for example on USB 0603:0002 mt touchpads.

This commit fixes this by only reporting non touch fields for the first
packet of a (possibly) multi-packet frame.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v4:
-New patch in this patch-set with a new approach focussed on only fixing
 the issues with the USB 0603:0002 mt touchpads
---
 drivers/hid/hid-multitouch.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 5d3904d0e89d..bc6a4f13c9ae 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -749,9 +749,11 @@ static int mt_touch_event(struct hid_device *hid, struct hid_field *field,
 }
 
 static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
-				struct hid_usage *usage, __s32 value)
+				struct hid_usage *usage, __s32 value,
+				bool first_packet)
 {
 	struct mt_device *td = hid_get_drvdata(hid);
+	__s32 cls = td->mtclass.name;
 	__s32 quirks = td->mtclass.quirks;
 	struct input_dev *input = field->hidinput->input;
 
@@ -805,6 +807,15 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
 			break;
 
 		default:
+			/*
+			 * For Win8 PTP touchpads we should only look at
+			 * non finger/touch events in the first_packet of
+			 * a (possible) multi-packet frame.
+			 */
+			if ((cls == MT_CLS_WIN_8 || cls == MT_CLS_WIN_8_DUAL) &&
+			    !first_packet)
+				return;
+
 			if (usage->type)
 				input_event(input, usage->type, usage->code,
 						value);
@@ -825,6 +836,7 @@ static void mt_touch_report(struct hid_device *hid, struct hid_report *report)
 	struct mt_device *td = hid_get_drvdata(hid);
 	__s32 cls = td->mtclass.name;
 	struct hid_field *field;
+	bool first_packet;
 	unsigned count;
 	int r, n, scantime = 0;
 
@@ -860,6 +872,7 @@ static void mt_touch_report(struct hid_device *hid, struct hid_report *report)
 	}
 	td->prev_scantime = scantime;
 
+	first_packet = td->num_received == 0;
 	for (r = 0; r < report->maxfield; r++) {
 		field = report->field[r];
 		count = field->report_count;
@@ -869,7 +882,7 @@ static void mt_touch_report(struct hid_device *hid, struct hid_report *report)
 
 		for (n = 0; n < count; n++)
 			mt_process_mt_event(hid, field, &field->usage[n],
-					field->value[n]);
+					    field->value[n], first_packet);
 	}
 
 	if (td->num_received >= td->num_expected)
-- 
2.14.3

[PATCH v4 4/4] HID: multitouch: Combine all left-button events in a frame

[Hans de Goede]

According to the Win8 Precision Touchpad spec, inside the HID_UP_BUTTON
usage-page usage 1 is for a clickpad getting clicked, 2 for an external
left button and 3 for an external right button. Since Linux uses
BTN_LEFT for a clickpad being clicked we end up mapping both usage 1
and 2 to BTN_LEFT and if a single report contains both then we ended
up always reporting the value of both in a single SYN, e.g. :
BTN_LEFT 1, BTN_LEFT 0, SYN. This happens for example with Hantick
HTT5288 i2c mt touchpads.

This commit fixes this by not immediately reporting left button when we
parse the report, but instead storing or-ing together the values and
reporting the result from mt_sync_frame() when we've a complete frame.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Rewrite of v1 of "HID: multitouch: Fix BTN_LEFT 0-1-0-1 events on Novatek
 mt touchpad" to kill two birds with one stone

Changes in v3:
-Delay reporting for all buttons not just for BTN_LEFT

Changes in v4:
-Back to combining only the value for BTN_LEFT, the other issues are fixed
 with the new "HID: multitouch: Only look at non touch fields in first
 packet of a frame" patch
---
 drivers/hid/hid-multitouch.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index bc6a4f13c9ae..00d4e32681b1 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -120,6 +120,7 @@ struct mt_device {
 	int scantime_index;	/* scantime field index in the report */
 	int scantime_val_index;	/* scantime value index in the field */
 	int prev_scantime;	/* scantime reported in the previous packet */
+	int left_button_state;	/* left button state */
 	unsigned last_slot_field;	/* the last field of a slot */
 	unsigned mt_report_id;	/* the report ID of the multitouch device */
 	unsigned long initial_quirks;	/* initial quirks state */
@@ -728,9 +729,15 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
  */
 static void mt_sync_frame(struct mt_device *td, struct input_dev *input)
 {
+	__s32 cls = td->mtclass.name;
+
+	if (cls == MT_CLS_WIN_8 || cls == MT_CLS_WIN_8_DUAL)
+		input_event(input, EV_KEY, BTN_LEFT, td->left_button_state);
+
 	input_mt_sync_frame(input);
 	input_sync(input);
 	td->num_received = 0;
+	td->left_button_state = 0;
 	if (test_bit(MT_IO_FLAGS_ACTIVE_SLOTS, &td->mt_io_flags))
 		set_bit(MT_IO_FLAGS_PENDING_SLOTS, &td->mt_io_flags);
 	else
@@ -816,6 +823,19 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
 			    !first_packet)
 				return;
 
+			/*
+			 * For Win8 PTP touchpads we map both the clickpad click
+			 * and any "external" left buttons to BTN_LEFT if a
+			 * device claims to have both we need to report 1 for
+			 * BTN_LEFT if either is pressed, so we or all values
+			 * together and report the result in mt_sync_frame().
+			 */
+			if ((cls == MT_CLS_WIN_8 || cls == MT_CLS_WIN_8_DUAL) &&
+			    usage->type == EV_KEY && usage->code == BTN_LEFT) {
+				td->left_button_state |= value;
+				return;
+			}
+
 			if (usage->type)
 				input_event(input, usage->type, usage->code,
 						value);
-- 
2.14.3

Re: [PATCH v4 4/4] HID: multitouch: Combine all left-button events in a frame

[Benjamin Tissoires]

On Nov 13 2017 or thereabouts, Hans de Goede wrote:
> According to the Win8 Precision Touchpad spec, inside the HID_UP_BUTTON
> usage-page usage 1 is for a clickpad getting clicked, 2 for an external
> left button and 3 for an external right button. Since Linux uses
> BTN_LEFT for a clickpad being clicked we end up mapping both usage 1
> and 2 to BTN_LEFT and if a single report contains both then we ended
> up always reporting the value of both in a single SYN, e.g. :
> BTN_LEFT 1, BTN_LEFT 0, SYN. This happens for example with Hantick
> HTT5288 i2c mt touchpads.
> 
> This commit fixes this by not immediately reporting left button when we
> parse the report, but instead storing or-ing together the values and
> reporting the result from mt_sync_frame() when we've a complete frame.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---

Thanks Hans for the re-spin of the series.
I think we are good now, series is:

Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

> Changes in v2:
> -Rewrite of v1 of "HID: multitouch: Fix BTN_LEFT 0-1-0-1 events on Novatek
>  mt touchpad" to kill two birds with one stone
> 
> Changes in v3:
> -Delay reporting for all buttons not just for BTN_LEFT
> 
> Changes in v4:
> -Back to combining only the value for BTN_LEFT, the other issues are fixed
>  with the new "HID: multitouch: Only look at non touch fields in first
>  packet of a frame" patch
> ---
>  drivers/hid/hid-multitouch.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index bc6a4f13c9ae..00d4e32681b1 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -120,6 +120,7 @@ struct mt_device {
>  	int scantime_index;	/* scantime field index in the report */
>  	int scantime_val_index;	/* scantime value index in the field */
>  	int prev_scantime;	/* scantime reported in the previous packet */
> +	int left_button_state;	/* left button state */
>  	unsigned last_slot_field;	/* the last field of a slot */
>  	unsigned mt_report_id;	/* the report ID of the multitouch device */
>  	unsigned long initial_quirks;	/* initial quirks state */
> @@ -728,9 +729,15 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>   */
>  static void mt_sync_frame(struct mt_device *td, struct input_dev *input)
>  {
> +	__s32 cls = td->mtclass.name;
> +
> +	if (cls == MT_CLS_WIN_8 || cls == MT_CLS_WIN_8_DUAL)
> +		input_event(input, EV_KEY, BTN_LEFT, td->left_button_state);
> +
>  	input_mt_sync_frame(input);
>  	input_sync(input);
>  	td->num_received = 0;
> +	td->left_button_state = 0;
>  	if (test_bit(MT_IO_FLAGS_ACTIVE_SLOTS, &td->mt_io_flags))
>  		set_bit(MT_IO_FLAGS_PENDING_SLOTS, &td->mt_io_flags);
>  	else
> @@ -816,6 +823,19 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
>  			    !first_packet)
>  				return;
>  
> +			/*
> +			 * For Win8 PTP touchpads we map both the clickpad click
> +			 * and any "external" left buttons to BTN_LEFT if a
> +			 * device claims to have both we need to report 1 for
> +			 * BTN_LEFT if either is pressed, so we or all values
> +			 * together and report the result in mt_sync_frame().
> +			 */
> +			if ((cls == MT_CLS_WIN_8 || cls == MT_CLS_WIN_8_DUAL) &&
> +			    usage->type == EV_KEY && usage->code == BTN_LEFT) {
> +				td->left_button_state |= value;
> +				return;
> +			}
> +
>  			if (usage->type)
>  				input_event(input, usage->type, usage->code,
>  						value);
> -- 
> 2.14.3
> 

Re: [PATCH v4 4/4] HID: multitouch: Combine all left-button events in a frame

[Jiri Kosina]

On Tue, 14 Nov 2017, Benjamin Tissoires wrote:

> > According to the Win8 Precision Touchpad spec, inside the HID_UP_BUTTON
> > usage-page usage 1 is for a clickpad getting clicked, 2 for an external
> > left button and 3 for an external right button. Since Linux uses
> > BTN_LEFT for a clickpad being clicked we end up mapping both usage 1
> > and 2 to BTN_LEFT and if a single report contains both then we ended
> > up always reporting the value of both in a single SYN, e.g. :
> > BTN_LEFT 1, BTN_LEFT 0, SYN. This happens for example with Hantick
> > HTT5288 i2c mt touchpads.
> > 
> > This commit fixes this by not immediately reporting left button when we
> > parse the report, but instead storing or-ing together the values and
> > reporting the result from mt_sync_frame() when we've a complete frame.
> > 
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> 
> Thanks Hans for the re-spin of the series.
> I think we are good now, series is:
> 
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Thanks. Hans, last request for you -- HID_DG_SCANTIME handling has in the 
meantime added for forwarding the MSC_TIMESTAMP (commit 29cc309d8bf1). 
Could you please respin your patch on top of that (basically just merge 
your handling with the one added for MSC_TIMESTAMP) and resend? I'll then 
queue it for 4.16. Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v4 4/4] HID: multitouch: Combine all left-button events in a frame

[Hans de Goede]

Hi,

On 21-11-17 13:11, Jiri Kosina wrote:
> On Tue, 14 Nov 2017, Benjamin Tissoires wrote:
> 
>>> According to the Win8 Precision Touchpad spec, inside the HID_UP_BUTTON
>>> usage-page usage 1 is for a clickpad getting clicked, 2 for an external
>>> left button and 3 for an external right button. Since Linux uses
>>> BTN_LEFT for a clickpad being clicked we end up mapping both usage 1
>>> and 2 to BTN_LEFT and if a single report contains both then we ended
>>> up always reporting the value of both in a single SYN, e.g. :
>>> BTN_LEFT 1, BTN_LEFT 0, SYN. This happens for example with Hantick
>>> HTT5288 i2c mt touchpads.
>>>
>>> This commit fixes this by not immediately reporting left button when we
>>> parse the report, but instead storing or-ing together the values and
>>> reporting the result from mt_sync_frame() when we've a complete frame.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>
>> Thanks Hans for the re-spin of the series.
>> I think we are good now, series is:
>>
>> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> Thanks. Hans, last request for you -- HID_DG_SCANTIME handling has in the
> meantime added for forwarding the MSC_TIMESTAMP (commit 29cc309d8bf1).
> Could you please respin your patch on top of that (basically just merge
> your handling with the one added for MSC_TIMESTAMP) and resend? I'll then
> queue it for 4.16. Thanks,

Ok, v5 with this fixed coming up.

Since this fixes 2 touchpad models not working (not being usable at
least) it would be nice to get this series into 4.15 rc2/rc3 as a
bugfix IMHO.

Regards,

Hans

Re: [PATCH v4 4/4] HID: multitouch: Combine all left-button events in a frame

[Jiri Kosina]

On Wed, 22 Nov 2017, Hans de Goede wrote:

> > Thanks. Hans, last request for you -- HID_DG_SCANTIME handling has in the
> > meantime added for forwarding the MSC_TIMESTAMP (commit 29cc309d8bf1).
> > Could you please respin your patch on top of that (basically just merge
> > your handling with the one added for MSC_TIMESTAMP) and resend? I'll then
> > queue it for 4.16. Thanks,
> 
> Ok, v5 with this fixed coming up.

Thanks a lot Hans.

> Since this fixes 2 touchpad models not working (not being usable at 
> least) it would be nice to get this series into 4.15 rc2/rc3 as a bugfix 
> IMHO.

Hmm, as this is changing common code paths, I'd prefer not to push it 
before next merge window really.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v4 4/4] HID: multitouch: Combine all left-button events in a frame

[Hans de Goede]

Hi,

On 22-11-17 14:37, Jiri Kosina wrote:
> On Wed, 22 Nov 2017, Hans de Goede wrote:
> 
>>> Thanks. Hans, last request for you -- HID_DG_SCANTIME handling has in the
>>> meantime added for forwarding the MSC_TIMESTAMP (commit 29cc309d8bf1).
>>> Could you please respin your patch on top of that (basically just merge
>>> your handling with the one added for MSC_TIMESTAMP) and resend? I'll then
>>> queue it for 4.16. Thanks,
>>
>> Ok, v5 with this fixed coming up.
> 
> Thanks a lot Hans.
> 
>> Since this fixes 2 touchpad models not working (not being usable at
>> least) it would be nice to get this series into 4.15 rc2/rc3 as a bugfix
>> IMHO.
> 
> Hmm, as this is changing common code paths, I'd prefer not to push it
> before next merge window really.

I understand, although we are still pre-rc1, so if you queue this up
for rc2 we won't miss a lot of the testing window, and I don't
think a lot of people with hid-mt touchpads are running -next...

Benjamin, what is your take on this?

Regards,

Hans

Re: [PATCH v4 4/4] HID: multitouch: Combine all left-button events in a frame

[Benjamin Tissoires]

On Nov 22 2017 or thereabouts, Hans de Goede wrote:
> Hi,
> 
> On 22-11-17 14:37, Jiri Kosina wrote:
> > On Wed, 22 Nov 2017, Hans de Goede wrote:
> > 
> > > > Thanks. Hans, last request for you -- HID_DG_SCANTIME handling has in the
> > > > meantime added for forwarding the MSC_TIMESTAMP (commit 29cc309d8bf1).
> > > > Could you please respin your patch on top of that (basically just merge
> > > > your handling with the one added for MSC_TIMESTAMP) and resend? I'll then
> > > > queue it for 4.16. Thanks,
> > > 
> > > Ok, v5 with this fixed coming up.
> > 
> > Thanks a lot Hans.
> > 
> > > Since this fixes 2 touchpad models not working (not being usable at
> > > least) it would be nice to get this series into 4.15 rc2/rc3 as a bugfix
> > > IMHO.
> > 
> > Hmm, as this is changing common code paths, I'd prefer not to push it
> > before next merge window really.
> 
> I understand, although we are still pre-rc1, so if you queue this up
> for rc2 we won't miss a lot of the testing window, and I don't
> think a lot of people with hid-mt touchpads are running -next...
> 
> Benjamin, what is your take on this?

Well, I think both arguments are valid. I would think the patch series
is safe, but we never know.
How about we schedule this for 4.16 and get it merged in Fedora
27/rawhide soon, so it gets the testings you want but is still scheduled
upstream?

Cheers,
Benjamin

> 
> Regards,
> 
> Hans

Re: [PATCH v4 4/4] HID: multitouch: Combine all left-button events in a frame

[Hans de Goede]

Hi,

On 22-11-17 14:59, Benjamin Tissoires wrote:
> On Nov 22 2017 or thereabouts, Hans de Goede wrote:
>> Hi,
>>
>> On 22-11-17 14:37, Jiri Kosina wrote:
>>> On Wed, 22 Nov 2017, Hans de Goede wrote:
>>>
>>>>> Thanks. Hans, last request for you -- HID_DG_SCANTIME handling has in the
>>>>> meantime added for forwarding the MSC_TIMESTAMP (commit 29cc309d8bf1).
>>>>> Could you please respin your patch on top of that (basically just merge
>>>>> your handling with the one added for MSC_TIMESTAMP) and resend? I'll then
>>>>> queue it for 4.16. Thanks,
>>>>
>>>> Ok, v5 with this fixed coming up.
>>>
>>> Thanks a lot Hans.
>>>
>>>> Since this fixes 2 touchpad models not working (not being usable at
>>>> least) it would be nice to get this series into 4.15 rc2/rc3 as a bugfix
>>>> IMHO.
>>>
>>> Hmm, as this is changing common code paths, I'd prefer not to push it
>>> before next merge window really.
>>
>> I understand, although we are still pre-rc1, so if you queue this up
>> for rc2 we won't miss a lot of the testing window, and I don't
>> think a lot of people with hid-mt touchpads are running -next...
>>
>> Benjamin, what is your take on this?
> 
> Well, I think both arguments are valid. I would think the patch series
> is safe, but we never know.
> How about we schedule this for 4.16 and get it merged in Fedora
> 27/rawhide soon, so it gets the testings you want but is still scheduled
> upstream?

You mean add it to the 4.15 kernels in rawhide? That would work for me.

Regards,

Hans

Re: [PATCH v4 4/4] HID: multitouch: Combine all left-button events in a frame

[Benjamin Tissoires]

On Nov 22 2017 or thereabouts, Hans de Goede wrote:
> Hi,
> 
> On 22-11-17 14:59, Benjamin Tissoires wrote:
> > On Nov 22 2017 or thereabouts, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 22-11-17 14:37, Jiri Kosina wrote:
> > > > On Wed, 22 Nov 2017, Hans de Goede wrote:
> > > > 
> > > > > > Thanks. Hans, last request for you -- HID_DG_SCANTIME handling has in the
> > > > > > meantime added for forwarding the MSC_TIMESTAMP (commit 29cc309d8bf1).
> > > > > > Could you please respin your patch on top of that (basically just merge
> > > > > > your handling with the one added for MSC_TIMESTAMP) and resend? I'll then
> > > > > > queue it for 4.16. Thanks,
> > > > > 
> > > > > Ok, v5 with this fixed coming up.
> > > > 
> > > > Thanks a lot Hans.
> > > > 
> > > > > Since this fixes 2 touchpad models not working (not being usable at
> > > > > least) it would be nice to get this series into 4.15 rc2/rc3 as a bugfix
> > > > > IMHO.
> > > > 
> > > > Hmm, as this is changing common code paths, I'd prefer not to push it
> > > > before next merge window really.
> > > 
> > > I understand, although we are still pre-rc1, so if you queue this up
> > > for rc2 we won't miss a lot of the testing window, and I don't
> > > think a lot of people with hid-mt touchpads are running -next...
> > > 
> > > Benjamin, what is your take on this?
> > 
> > Well, I think both arguments are valid. I would think the patch series
> > is safe, but we never know.
> > How about we schedule this for 4.16 and get it merged in Fedora
> > 27/rawhide soon, so it gets the testings you want but is still scheduled
> > upstream?
> 
> You mean add it to the 4.15 kernels in rawhide? That would work for me.

Yes. Reverting a broken patch in Fedora is easy enough and given that
the patch is accepted upstream, it's not like it will be a big
difference from upstream.

Cheers,
Benjamin

> 
> Regards,
> 
> Hans