diff mbox

[1/6] HID: wacom: generic: Send MSC_SERIAL and ABS_MISC when leaving prox

Message ID 20170908004412.2698-1-ping.cheng@wacom.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ping Cheng Sept. 8, 2017, 12:44 a.m. UTC 
From: Jason Gerecke <killertofu@gmail.com>

The latest generation of pro devices (MobileStudio Pro, 2nd-gen Intuos
Pro, Cintiq Pro) send a serial number of '0' whenever the pen is too far
away for reliable communication. Userspace defines that a serial number
of '0' is invalid, so we need to be careful not to actually forward
this value. Additionally, since EMR ISDv4 devices do not support serial
numbers or tool IDs, we'd like to not send these events if they aren't
necessary.

The existing code achieves these goals by adding a check for a non-zero
serial number within the wacom_wac_pen_report function. The MSC_SERIAL
and ABS_MISC events are only sent if the serial number is non-zero. This
code fails, however when the pen for a pro device leaves proximity. When
the pen leaves prox and the tablet sends a serial of 0, wacom_wac_pen_event
dutifully clears the serial number. When wacom_wac_pen_report is called,
it does not send either the MSC_SERIAL of the exiting tool nor an ABS_MISC
event.

This patch prevents the wacom_wac_pen_event function from clearing an
already-set serial number. This ensures that we have the serial number
handy when exiting proximity, but requires us to manually clear it
afterwards to ensure the driver does not send stale data (e.g. when
switching between AES pens that report a serial nubmer of 0 for the
first few fully in-proximity packets).

Fixes: f85c9dc678 ("HID: wacom: generic: Support tool ID and additional tool types")
Cc: stable # v4.10 <stable@vger.kernel.org>
Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
 drivers/hid/wacom_wac.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

Comments

Jiri Kosina Sept. 8, 2017, 12:14 p.m. UTC | #1 
Hi,

so, my plan with this patchset is to push first two patches into 4.14-rc 
as bufixes, and queue patches 3-6 for next merge window (4.15). Please let 
me know in case you disagree.

Thanks,
Ping Cheng Sept. 8, 2017, 10:09 p.m. UTC | #2 
Hi Jiri,

On Fri, Sep 8, 2017 at 5:14 AM, Jiri Kosina <jikos@kernel.org> wrote:
> Hi,
>
> so, my plan with this patchset is to push first two patches into 4.14-rc
> as bufixes, and queue patches 3-6 for next merge window (4.15). Please let
> me know in case you disagree.

This set of patches were triggered by Bug 196673
(https://bugzilla.kernel.org/show_bug.cgi?id=196673). First two
patches fix patch f85c9dc678 ("HID: wacom: generic: Support tool ID
and additional tool types"). But, they may not be enough to fix Bug
196673. We are waiting for a confirmation from Hrvoje Zeba.

If you can't push all 6 into 4.14-rc, just the first 2 would be fine.

Cheers,
Ping
--
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
Jiri Kosina Sept. 11, 2017, 8:51 a.m. UTC | #3 
On Fri, 8 Sep 2017, Ping Cheng wrote:

> > so, my plan with this patchset is to push first two patches into 4.14-rc
> > as bufixes, and queue patches 3-6 for next merge window (4.15). Please let
> > me know in case you disagree.
> 
> This set of patches were triggered by Bug 196673
> (https://bugzilla.kernel.org/show_bug.cgi?id=196673). First two
> patches fix patch f85c9dc678 ("HID: wacom: generic: Support tool ID
> and additional tool types"). But, they may not be enough to fix Bug
> 196673. We are waiting for a confirmation from Hrvoje Zeba.
> 
> If you can't push all 6 into 4.14-rc, just the first 2 would be fine.

I'd like to merge the minimal necessary set required to fix the 
regression, and queue the palm detection / proximity improvements for 
merge window if they are not necessary.

But if you figure out that all 6 patches are needed to fix the regression, 
I'll of course push the full set.

I'll be waiting for your confirmation. Thanks,

> 
> Cheers,
> Ping
>
Ping Cheng Sept. 11, 2017, 11:57 p.m. UTC | #4 
On Mon, Sep 11, 2017 at 1:51 AM, Jiri Kosina <jikos@kernel.org> wrote:
> On Fri, 8 Sep 2017, Ping Cheng wrote:
>
>> > so, my plan with this patchset is to push first two patches into 4.14-rc
>> > as bufixes, and queue patches 3-6 for next merge window (4.15). Please let
>> > me know in case you disagree.
>>
>> This set of patches were triggered by Bug 196673
>> (https://bugzilla.kernel.org/show_bug.cgi?id=196673). First two
>> patches fix patch f85c9dc678 ("HID: wacom: generic: Support tool ID
>> and additional tool types"). But, they may not be enough to fix Bug
>> 196673. We are waiting for a confirmation from Hrvoje Zeba.
>>
>> If you can't push all 6 into 4.14-rc, just the first 2 would be fine.
>
> I'd like to merge the minimal necessary set required to fix the
> regression, and queue the palm detection / proximity improvements for
> merge window if they are not necessary.
>
> But if you figure out that all 6 patches are needed to fix the regression,
> I'll of course push the full set.
>
> I'll be waiting for your confirmation. Thanks,

Only the first 2 need to go in 4.14-rc. The other 4 can wait until
4.15 is open, as you suggested.

Thank you Jiri.

Ping
--
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
Jiri Kosina Sept. 13, 2017, 5:18 p.m. UTC | #5 
On Mon, 11 Sep 2017, Ping Cheng wrote:

> Only the first 2 need to go in 4.14-rc. The other 4 can wait until
> 4.15 is open, as you suggested.

Thanks a lot for verifying. Applied the first two to 
for-4.14/upstream-fixes and the rest on top into for-4.15/wacom
diff mbox

Patch

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 78d0398..e2ba36d 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -2141,8 +2141,10 @@  static void wacom_wac_pen_event(struct hid_device *hdev, struct hid_field *field
 		wacom_wac->hid_data.tipswitch |= value;
 		return;
 	case HID_DG_TOOLSERIALNUMBER:
-		wacom_wac->serial[0] = (wacom_wac->serial[0] & ~0xFFFFFFFFULL);
-		wacom_wac->serial[0] |= (__u32)value;
+		if (value) {
+			wacom_wac->serial[0] = (wacom_wac->serial[0] & ~0xFFFFFFFFULL);
+			wacom_wac->serial[0] |= (__u32)value;
+		}
 		return;
 	case HID_DG_TWIST:
 		/*
@@ -2156,15 +2158,17 @@  static void wacom_wac_pen_event(struct hid_device *hdev, struct hid_field *field
 		wacom_wac->hid_data.sense_state = value;
 		return;
 	case WACOM_HID_WD_SERIALHI:
-		wacom_wac->serial[0] = (wacom_wac->serial[0] & 0xFFFFFFFF);
-		wacom_wac->serial[0] |= ((__u64)value) << 32;
-		/*
-		 * Non-USI EMR devices may contain additional tool type
-		 * information here. See WACOM_HID_WD_TOOLTYPE case for
-		 * more details.
-		 */
-		if (value >> 20 == 1) {
-			wacom_wac->id[0] |= value & 0xFFFFF;
+		if (value) {
+			wacom_wac->serial[0] = (wacom_wac->serial[0] & 0xFFFFFFFF);
+			wacom_wac->serial[0] |= ((__u64)value) << 32;
+			/*
+			 * Non-USI EMR devices may contain additional tool type
+			 * information here. See WACOM_HID_WD_TOOLTYPE case for
+			 * more details.
+			 */
+			if (value >> 20 == 1) {
+				wacom_wac->id[0] |= value & 0xFFFFF;
+			}
 		}
 		return;
 	case WACOM_HID_WD_TOOLTYPE:
@@ -2279,6 +2283,7 @@  static void wacom_wac_pen_report(struct hid_device *hdev,
 	if (!prox) {
 		wacom_wac->tool[0] = 0;
 		wacom_wac->id[0] = 0;
+		wacom_wac->serial[0] = 0;
 	}
 }