[PATCH] HID: wacom: Support switching from vendor-defined input format on G9 and G11

[PATCH] HID: wacom: Support switching from vendor-defined input format on G9 and G11

[Jason Gerecke]

A tablet PC booted into Windows may have its pen/touch hardware switched
into "Wacom mode" similar to what we do with explicitly-supported hardware.
Some devices appear to maintain this state across reboots, preventing their
use with the generic HID driver. This patch adds support for the vendor-
defined "input format" feature report of G9 and G11 chips and has the HID
codepath always attempt to reset the device back to an appropriate format.

Fixes: https://sourceforge.net/p/linuxwacom/bugs/307/
Fixes: https://sourceforge.net/p/linuxwacom/bugs/310/
Fixes: https://github.com/linuxwacom/input-wacom/issues/15

Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
 drivers/hid/wacom_sys.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++--
 drivers/hid/wacom_wac.h | 13 ++++++++++--
 2 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 68a5609..d7ef7b0 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -152,6 +152,32 @@ static void wacom_feature_mapping(struct hid_device *hdev,
 		hid_data->inputmode = field->report->id;
 		hid_data->inputmode_index = usage->usage_index;
 		break;
+
+	/* The vendor-defined application collections leave most usages
+	 * as null (0x0000), making them much more difficult to match...
+	 */
+	case HID_UP_DIGITIZER:
+		if (field->application == WACOM_G9_DIGITIZER ||
+		    field->application == WACOM_G11_DIGITIZER) {
+			if (field->report->id == 0x0B) {
+				hid_data->inputformat = field->report->id;
+				hid_data->inputformat_index = 0;
+				hid_data->inputformat_value = 0; /* 0: HID; 1: Vendor-defined */
+			}
+		}
+		break;
+
+	case WACOM_G9_PAGE:
+	case WACOM_G11_PAGE:
+		if (field->application == WACOM_G9_TOUCHSCREEN ||
+		    field->application == WACOM_G11_TOUCHSCREEN) {
+			if (field->report->id == 0x03) {
+				hid_data->inputformat = field->report->id;
+				hid_data->inputformat_index = 0;
+				hid_data->inputformat_value = 0; /* 0: HID; 4: Vendor-defined */
+			}
+		}
+		break;
 	}
 }
 
@@ -322,6 +348,25 @@ static int wacom_hid_set_device_mode(struct hid_device *hdev)
 	return 0;
 }
 
+static int wacom_hid_set_device_format(struct hid_device *hdev)
+{
+	struct wacom *wacom = hid_get_drvdata(hdev);
+	struct hid_data *hid_data = &wacom->wacom_wac.hid_data;
+	struct hid_report *r;
+	struct hid_report_enum *re;
+
+	if (hid_data->inputformat < 0)
+		return 0;
+
+	re = &(hdev->report_enum[HID_FEATURE_REPORT]);
+	r = re->report_id_hash[hid_data->inputformat];
+	if (r) {
+		r->field[0]->value[hid_data->inputformat_index] = hid_data->inputformat_value;
+		hid_hw_request(hdev, r, HID_REQ_SET_REPORT);
+	}
+	return 0;
+}
+
 static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
 		int length, int mode)
 {
@@ -414,8 +459,14 @@ static int wacom_query_tablet_data(struct hid_device *hdev,
 	if (hdev->bus == BUS_BLUETOOTH)
 		return wacom_bt_query_tablet_data(hdev, 1, features);
 
-	if (features->type == HID_GENERIC)
-		return wacom_hid_set_device_mode(hdev);
+	if (features->type == HID_GENERIC) {
+		int err;
+		err = wacom_hid_set_device_mode(hdev);
+		if (err)
+			return err;
+		err = wacom_hid_set_device_format(hdev);
+		return err;
+	}
 
 	if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
 		if (features->type > TABLETPC) {
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 25baa7f..37d5dec 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -84,6 +84,12 @@
 #define WACOM_DEVICETYPE_WL_MONITOR     0x0008
 
 #define WACOM_VENDORDEFINED_PEN		0xff0d0001
+#define WACOM_G9_PAGE			0xff090000
+#define WACOM_G9_DIGITIZER		(WACOM_G9_PAGE | 0x02)
+#define WACOM_G9_TOUCHSCREEN		(WACOM_G9_PAGE | 0x11)
+#define WACOM_G11_PAGE			0xff110000
+#define WACOM_G11_DIGITIZER		(WACOM_G11_PAGE | 0x02)
+#define WACOM_G11_TOUCHSCREEN		(WACOM_G11_PAGE | 0x11)
 
 #define WACOM_PEN_FIELD(f)	(((f)->logical == HID_DG_STYLUS) || \
 				 ((f)->physical == HID_DG_STYLUS) || \
@@ -193,8 +199,11 @@ struct wacom_shared {
 };
 
 struct hid_data {
-	__s16 inputmode;	/* InputMode HID feature, -1 if non-existent */
-	__s16 inputmode_index;	/* InputMode HID feature index in the report */
+	__s16 inputmode;		/* InputMode HID feature, -1 if non-existent */
+	__s16 inputmode_index;		/* InputMode HID feature index in the report */
+	__s16 inputformat;		/* Vendor-defined "input format" feature, -1 if non-existent */
+	__s16 inputformat_index;	/* Vendor-defined "input format" feature index in the report */
+	__u8  inputformat_value;	/* Value expected by device to switch to HID mode */
 	bool inrange_state;
 	bool invert_state;
 	bool tipswitch;
-- 
2.7.3

Re: [PATCH] HID: wacom: Support switching from vendor-defined input format on G9 and G11

[Benjamin Tissoires]

Hi Jason,

On Mar 28 2016 or thereabouts, Jason Gerecke wrote:
> A tablet PC booted into Windows may have its pen/touch hardware switched
> into "Wacom mode" similar to what we do with explicitly-supported hardware.
> Some devices appear to maintain this state across reboots, preventing their
> use with the generic HID driver. This patch adds support for the vendor-
> defined "input format" feature report of G9 and G11 chips and has the HID
> codepath always attempt to reset the device back to an appropriate format.
> 
> Fixes: https://sourceforge.net/p/linuxwacom/bugs/307/
> Fixes: https://sourceforge.net/p/linuxwacom/bugs/310/
> Fixes: https://github.com/linuxwacom/input-wacom/issues/15
> 
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> ---
>  drivers/hid/wacom_sys.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++--
>  drivers/hid/wacom_wac.h | 13 ++++++++++--
>  2 files changed, 64 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 68a5609..d7ef7b0 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -152,6 +152,32 @@ static void wacom_feature_mapping(struct hid_device *hdev,
>  		hid_data->inputmode = field->report->id;
>  		hid_data->inputmode_index = usage->usage_index;
>  		break;
> +
> +	/* The vendor-defined application collections leave most usages
> +	 * as null (0x0000), making them much more difficult to match...
> +	 */

Not so sure this comments adds anything to the code. The application
usage is well defined, so there is no need to worry I think.

(Plus the coding style should be:
/*
 * multi lines
 * comment.
 */
)

> +	case HID_UP_DIGITIZER:
> +		if (field->application == WACOM_G9_DIGITIZER ||
> +		    field->application == WACOM_G11_DIGITIZER) {
> +			if (field->report->id == 0x0B) {

I'd rather have the test on the report id first, and have only one if
(with id && (app || app). This way, you'll keep the extra tab below.

> +				hid_data->inputformat = field->report->id;
> +				hid_data->inputformat_index = 0;

Shouldn't this be usage->usage_index instead? Unless it's defined
several times, which would make the use of usage_index moot.

> +				hid_data->inputformat_value = 0; /* 0: HID; 1: Vendor-defined */

Looks like both usages are requesting this to be set at 0. I think it's
fine to keep it, but I don't know if it's that important to keep then.

> +			}
> +		}
> +		break;
> +
> +	case WACOM_G9_PAGE:
> +	case WACOM_G11_PAGE:
> +		if (field->application == WACOM_G9_TOUCHSCREEN ||
> +		    field->application == WACOM_G11_TOUCHSCREEN) {
> +			if (field->report->id == 0x03) {

Same three comments above apply here.

> +				hid_data->inputformat = field->report->id;
> +				hid_data->inputformat_index = 0;
> +				hid_data->inputformat_value = 0; /* 0: HID; 4: Vendor-defined */
> +			}
> +		}
> +		break;
>  	}
>  }
>  
> @@ -322,6 +348,25 @@ static int wacom_hid_set_device_mode(struct hid_device *hdev)
>  	return 0;
>  }
>  
> +static int wacom_hid_set_device_format(struct hid_device *hdev)
> +{
> +	struct wacom *wacom = hid_get_drvdata(hdev);
> +	struct hid_data *hid_data = &wacom->wacom_wac.hid_data;
> +	struct hid_report *r;
> +	struct hid_report_enum *re;
> +
> +	if (hid_data->inputformat < 0)
> +		return 0;
> +
> +	re = &(hdev->report_enum[HID_FEATURE_REPORT]);
> +	r = re->report_id_hash[hid_data->inputformat];
> +	if (r) {
> +		r->field[0]->value[hid_data->inputformat_index] = hid_data->inputformat_value;
> +		hid_hw_request(hdev, r, HID_REQ_SET_REPORT);
> +	}
> +	return 0;
> +}
> +
>  static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
>  		int length, int mode)
>  {
> @@ -414,8 +459,14 @@ static int wacom_query_tablet_data(struct hid_device *hdev,
>  	if (hdev->bus == BUS_BLUETOOTH)
>  		return wacom_bt_query_tablet_data(hdev, 1, features);
>  
> -	if (features->type == HID_GENERIC)
> -		return wacom_hid_set_device_mode(hdev);
> +	if (features->type == HID_GENERIC) {
> +		int err;
> +		err = wacom_hid_set_device_mode(hdev);
> +		if (err)
> +			return err;
> +		err = wacom_hid_set_device_format(hdev);

I don't really see the difference between "input mode" and "input
format". Does the device needs both to be set? If not, couldn't we
extend the input mode feature for these usages?

Cheers,
Benjamin

> +		return err;
> +	}
>  
>  	if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
>  		if (features->type > TABLETPC) {
> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> index 25baa7f..37d5dec 100644
> --- a/drivers/hid/wacom_wac.h
> +++ b/drivers/hid/wacom_wac.h
> @@ -84,6 +84,12 @@
>  #define WACOM_DEVICETYPE_WL_MONITOR     0x0008
>  
>  #define WACOM_VENDORDEFINED_PEN		0xff0d0001
> +#define WACOM_G9_PAGE			0xff090000
> +#define WACOM_G9_DIGITIZER		(WACOM_G9_PAGE | 0x02)
> +#define WACOM_G9_TOUCHSCREEN		(WACOM_G9_PAGE | 0x11)
> +#define WACOM_G11_PAGE			0xff110000
> +#define WACOM_G11_DIGITIZER		(WACOM_G11_PAGE | 0x02)
> +#define WACOM_G11_TOUCHSCREEN		(WACOM_G11_PAGE | 0x11)
>  
>  #define WACOM_PEN_FIELD(f)	(((f)->logical == HID_DG_STYLUS) || \
>  				 ((f)->physical == HID_DG_STYLUS) || \
> @@ -193,8 +199,11 @@ struct wacom_shared {
>  };
>  
>  struct hid_data {
> -	__s16 inputmode;	/* InputMode HID feature, -1 if non-existent */
> -	__s16 inputmode_index;	/* InputMode HID feature index in the report */
> +	__s16 inputmode;		/* InputMode HID feature, -1 if non-existent */
> +	__s16 inputmode_index;		/* InputMode HID feature index in the report */
> +	__s16 inputformat;		/* Vendor-defined "input format" feature, -1 if non-existent */
> +	__s16 inputformat_index;	/* Vendor-defined "input format" feature index in the report */
> +	__u8  inputformat_value;	/* Value expected by device to switch to HID mode */
>  	bool inrange_state;
>  	bool invert_state;
>  	bool tipswitch;
> -- 
> 2.7.3
> 

Re: [PATCH] HID: wacom: Support switching from vendor-defined input format on G9 and G11

[Jason Gerecke]

On 03/29/2016 12:05 AM, Benjamin Tissoires wrote:
> Hi Jason,
> 
> On Mar 28 2016 or thereabouts, Jason Gerecke wrote:
>> A tablet PC booted into Windows may have its pen/touch hardware switched
>> into "Wacom mode" similar to what we do with explicitly-supported hardware.
>> Some devices appear to maintain this state across reboots, preventing their
>> use with the generic HID driver. This patch adds support for the vendor-
>> defined "input format" feature report of G9 and G11 chips and has the HID
>> codepath always attempt to reset the device back to an appropriate format.
>>
>> Fixes: https://sourceforge.net/p/linuxwacom/bugs/307/
>> Fixes: https://sourceforge.net/p/linuxwacom/bugs/310/
>> Fixes: https://github.com/linuxwacom/input-wacom/issues/15
>>
>> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
>> ---
>>  drivers/hid/wacom_sys.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++--
>>  drivers/hid/wacom_wac.h | 13 ++++++++++--
>>  2 files changed, 64 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
>> index 68a5609..d7ef7b0 100644
>> --- a/drivers/hid/wacom_sys.c
>> +++ b/drivers/hid/wacom_sys.c
>> @@ -152,6 +152,32 @@ static void wacom_feature_mapping(struct hid_device *hdev,
>>  		hid_data->inputmode = field->report->id;
>>  		hid_data->inputmode_index = usage->usage_index;
>>  		break;
>> +
>> +	/* The vendor-defined application collections leave most usages
>> +	 * as null (0x0000), making them much more difficult to match...
>> +	 */
> 
> Not so sure this comments adds anything to the code. The application
> usage is well defined, so there is no need to worry I think.
> 
> (Plus the coding style should be:
> /*
>  * multi lines
>  * comment.
>  */
> )
> 
>> +	case HID_UP_DIGITIZER:
>> +		if (field->application == WACOM_G9_DIGITIZER ||
>> +		    field->application == WACOM_G11_DIGITIZER) {
>> +			if (field->report->id == 0x0B) {
> 
> I'd rather have the test on the report id first, and have only one if
> (with id && (app || app). This way, you'll keep the extra tab below.
> 
>> +				hid_data->inputformat = field->report->id;
>> +				hid_data->inputformat_index = 0;
> 
> Shouldn't this be usage->usage_index instead? Unless it's defined
> several times, which would make the use of usage_index moot.
> 

The field is the entire contents of one of Wacom's infamous "n
vendor-defined bytes" reports, so although the usage itself isn't
defined multiple times it does have a report count associated with it
(with the same net effect).

>> +				hid_data->inputformat_value = 0; /* 0: HID; 1: Vendor-defined */
> 
> Looks like both usages are requesting this to be set at 0. I think it's
> fine to keep it, but I don't know if it's that important to keep then.
> 
>> +			}
>> +		}
>> +		break;
>> +
>> +	case WACOM_G9_PAGE:
>> +	case WACOM_G11_PAGE:
>> +		if (field->application == WACOM_G9_TOUCHSCREEN ||
>> +		    field->application == WACOM_G11_TOUCHSCREEN) {
>> +			if (field->report->id == 0x03) {
> 
> Same three comments above apply here.
> 
>> +				hid_data->inputformat = field->report->id;
>> +				hid_data->inputformat_index = 0;
>> +				hid_data->inputformat_value = 0; /* 0: HID; 4: Vendor-defined */
>> +			}
>> +		}
>> +		break;
>>  	}
>>  }
>>  
>> @@ -322,6 +348,25 @@ static int wacom_hid_set_device_mode(struct hid_device *hdev)
>>  	return 0;
>>  }
>>  
>> +static int wacom_hid_set_device_format(struct hid_device *hdev)
>> +{
>> +	struct wacom *wacom = hid_get_drvdata(hdev);
>> +	struct hid_data *hid_data = &wacom->wacom_wac.hid_data;
>> +	struct hid_report *r;
>> +	struct hid_report_enum *re;
>> +
>> +	if (hid_data->inputformat < 0)
>> +		return 0;
>> +
>> +	re = &(hdev->report_enum[HID_FEATURE_REPORT]);
>> +	r = re->report_id_hash[hid_data->inputformat];
>> +	if (r) {
>> +		r->field[0]->value[hid_data->inputformat_index] = hid_data->inputformat_value;
>> +		hid_hw_request(hdev, r, HID_REQ_SET_REPORT);
>> +	}
>> +	return 0;
>> +}
>> +
>>  static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
>>  		int length, int mode)
>>  {
>> @@ -414,8 +459,14 @@ static int wacom_query_tablet_data(struct hid_device *hdev,
>>  	if (hdev->bus == BUS_BLUETOOTH)
>>  		return wacom_bt_query_tablet_data(hdev, 1, features);
>>  
>> -	if (features->type == HID_GENERIC)
>> -		return wacom_hid_set_device_mode(hdev);
>> +	if (features->type == HID_GENERIC) {
>> +		int err;
>> +		err = wacom_hid_set_device_mode(hdev);
>> +		if (err)
>> +			return err;
>> +		err = wacom_hid_set_device_format(hdev);
> 
> I don't really see the difference between "input mode" and "input
> format". Does the device needs both to be set? If not, couldn't we
> extend the input mode feature for these usages?
> 
> Cheers,
> Benjamin
> 

Device / Input mode is a standard HID feature expected to be present on
Windows 7-compatible touch devices which is used to switch a device
between sending mouse, single, or multi-touch reports[1]. The
'wacom_hid_set_device_mode' function right now takes care of switching
into multi-touch mode provided the hid_data->inputmode field has been set.

Input format on the other hand is a vendor-defined feature that is used
to switch Wacom devices (both touchscreens and tablets) between sending
standard HID reports or vendor-defined reports. Devices are supposed to
start up sending the former, but at least two tablet PCs have had
hardware that seems to remember being switched to the vendor-defined format.

If the format is explicitly switched to HID standard, we still should
probably set the mode as well since I don't think there's anything
implied about which mode the tablet should use after switching.

Relatedly, it's probably a good idea to rename 'wacom_set_device_mode'
or 'wacom_hid_set_device_mode' since they each set different a different
kind of "mode". It _might_ also be reasonable to rework
'wacom_set_device_mode' to be compatible with HID_GENERIC devices
(rather than introducing 'wacom_hid_set_device_format'), but keeping
compatibility the explicit devices might be tricky...

Off to work on V2...

[1]:
https://msdn.microsoft.com/en-us/library/windows/hardware/ff553739%28v=vs.85%29.aspx

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....

>> +		return err;
>> +	}
>>  
>>  	if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
>>  		if (features->type > TABLETPC) {
>> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
>> index 25baa7f..37d5dec 100644
>> --- a/drivers/hid/wacom_wac.h
>> +++ b/drivers/hid/wacom_wac.h
>> @@ -84,6 +84,12 @@
>>  #define WACOM_DEVICETYPE_WL_MONITOR     0x0008
>>  
>>  #define WACOM_VENDORDEFINED_PEN		0xff0d0001
>> +#define WACOM_G9_PAGE			0xff090000
>> +#define WACOM_G9_DIGITIZER		(WACOM_G9_PAGE | 0x02)
>> +#define WACOM_G9_TOUCHSCREEN		(WACOM_G9_PAGE | 0x11)
>> +#define WACOM_G11_PAGE			0xff110000
>> +#define WACOM_G11_DIGITIZER		(WACOM_G11_PAGE | 0x02)
>> +#define WACOM_G11_TOUCHSCREEN		(WACOM_G11_PAGE | 0x11)
>>  
>>  #define WACOM_PEN_FIELD(f)	(((f)->logical == HID_DG_STYLUS) || \
>>  				 ((f)->physical == HID_DG_STYLUS) || \
>> @@ -193,8 +199,11 @@ struct wacom_shared {
>>  };
>>  
>>  struct hid_data {
>> -	__s16 inputmode;	/* InputMode HID feature, -1 if non-existent */
>> -	__s16 inputmode_index;	/* InputMode HID feature index in the report */
>> +	__s16 inputmode;		/* InputMode HID feature, -1 if non-existent */
>> +	__s16 inputmode_index;		/* InputMode HID feature index in the report */
>> +	__s16 inputformat;		/* Vendor-defined "input format" feature, -1 if non-existent */
>> +	__s16 inputformat_index;	/* Vendor-defined "input format" feature index in the report */
>> +	__u8  inputformat_value;	/* Value expected by device to switch to HID mode */
>>  	bool inrange_state;
>>  	bool invert_state;
>>  	bool tipswitch;
>> -- 
>> 2.7.3
>>
--
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] HID: wacom: Support switching from vendor-defined input format on G9 and G11

[Benjamin Tissoires]

On Mar 29 2016 or thereabouts, Jason Gerecke wrote:
> On 03/29/2016 12:05 AM, Benjamin Tissoires wrote:
> > Hi Jason,
> > 
> > On Mar 28 2016 or thereabouts, Jason Gerecke wrote:
> >> A tablet PC booted into Windows may have its pen/touch hardware switched
> >> into "Wacom mode" similar to what we do with explicitly-supported hardware.
> >> Some devices appear to maintain this state across reboots, preventing their
> >> use with the generic HID driver. This patch adds support for the vendor-
> >> defined "input format" feature report of G9 and G11 chips and has the HID
> >> codepath always attempt to reset the device back to an appropriate format.
> >>
> >> Fixes: https://sourceforge.net/p/linuxwacom/bugs/307/
> >> Fixes: https://sourceforge.net/p/linuxwacom/bugs/310/
> >> Fixes: https://github.com/linuxwacom/input-wacom/issues/15
> >>
> >> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> >> ---
> >>  drivers/hid/wacom_sys.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++--
> >>  drivers/hid/wacom_wac.h | 13 ++++++++++--
> >>  2 files changed, 64 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> >> index 68a5609..d7ef7b0 100644
> >> --- a/drivers/hid/wacom_sys.c
> >> +++ b/drivers/hid/wacom_sys.c
> >> @@ -152,6 +152,32 @@ static void wacom_feature_mapping(struct hid_device *hdev,
> >>  		hid_data->inputmode = field->report->id;
> >>  		hid_data->inputmode_index = usage->usage_index;
> >>  		break;
> >> +
> >> +	/* The vendor-defined application collections leave most usages
> >> +	 * as null (0x0000), making them much more difficult to match...
> >> +	 */
> > 
> > Not so sure this comments adds anything to the code. The application
> > usage is well defined, so there is no need to worry I think.
> > 
> > (Plus the coding style should be:
> > /*
> >  * multi lines
> >  * comment.
> >  */
> > )
> > 
> >> +	case HID_UP_DIGITIZER:
> >> +		if (field->application == WACOM_G9_DIGITIZER ||
> >> +		    field->application == WACOM_G11_DIGITIZER) {
> >> +			if (field->report->id == 0x0B) {
> > 
> > I'd rather have the test on the report id first, and have only one if
> > (with id && (app || app). This way, you'll keep the extra tab below.
> > 
> >> +				hid_data->inputformat = field->report->id;
> >> +				hid_data->inputformat_index = 0;
> > 
> > Shouldn't this be usage->usage_index instead? Unless it's defined
> > several times, which would make the use of usage_index moot.
> > 
> 
> The field is the entire contents of one of Wacom's infamous "n
> vendor-defined bytes" reports, so although the usage itself isn't
> defined multiple times it does have a report count associated with it
> (with the same net effect).

Fair enough. But I wonder if we need to keep the inputformat_index field
given that it will be always 0.

> 
> >> +				hid_data->inputformat_value = 0; /* 0: HID; 1: Vendor-defined */
> > 
> > Looks like both usages are requesting this to be set at 0. I think it's
> > fine to keep it, but I don't know if it's that important to keep then.
> > 
> >> +			}
> >> +		}
> >> +		break;
> >> +
> >> +	case WACOM_G9_PAGE:
> >> +	case WACOM_G11_PAGE:
> >> +		if (field->application == WACOM_G9_TOUCHSCREEN ||
> >> +		    field->application == WACOM_G11_TOUCHSCREEN) {
> >> +			if (field->report->id == 0x03) {
> > 
> > Same three comments above apply here.
> > 
> >> +				hid_data->inputformat = field->report->id;
> >> +				hid_data->inputformat_index = 0;
> >> +				hid_data->inputformat_value = 0; /* 0: HID; 4: Vendor-defined */
> >> +			}
> >> +		}
> >> +		break;
> >>  	}
> >>  }
> >>  
> >> @@ -322,6 +348,25 @@ static int wacom_hid_set_device_mode(struct hid_device *hdev)
> >>  	return 0;
> >>  }
> >>  
> >> +static int wacom_hid_set_device_format(struct hid_device *hdev)
> >> +{
> >> +	struct wacom *wacom = hid_get_drvdata(hdev);
> >> +	struct hid_data *hid_data = &wacom->wacom_wac.hid_data;
> >> +	struct hid_report *r;
> >> +	struct hid_report_enum *re;
> >> +
> >> +	if (hid_data->inputformat < 0)
> >> +		return 0;
> >> +
> >> +	re = &(hdev->report_enum[HID_FEATURE_REPORT]);
> >> +	r = re->report_id_hash[hid_data->inputformat];
> >> +	if (r) {
> >> +		r->field[0]->value[hid_data->inputformat_index] = hid_data->inputformat_value;
> >> +		hid_hw_request(hdev, r, HID_REQ_SET_REPORT);
> >> +	}
> >> +	return 0;
> >> +}
> >> +
> >>  static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
> >>  		int length, int mode)
> >>  {
> >> @@ -414,8 +459,14 @@ static int wacom_query_tablet_data(struct hid_device *hdev,
> >>  	if (hdev->bus == BUS_BLUETOOTH)
> >>  		return wacom_bt_query_tablet_data(hdev, 1, features);
> >>  
> >> -	if (features->type == HID_GENERIC)
> >> -		return wacom_hid_set_device_mode(hdev);
> >> +	if (features->type == HID_GENERIC) {
> >> +		int err;
> >> +		err = wacom_hid_set_device_mode(hdev);
> >> +		if (err)
> >> +			return err;
> >> +		err = wacom_hid_set_device_format(hdev);
> > 
> > I don't really see the difference between "input mode" and "input
> > format". Does the device needs both to be set? If not, couldn't we
> > extend the input mode feature for these usages?
> > 
> > Cheers,
> > Benjamin
> > 
> 
> Device / Input mode is a standard HID feature expected to be present on
> Windows 7-compatible touch devices which is used to switch a device
> between sending mouse, single, or multi-touch reports[1]. The
> 'wacom_hid_set_device_mode' function right now takes care of switching
> into multi-touch mode provided the hid_data->inputmode field has been set.

Well, "Device Mode" is also used by MS for Pen devices (values are 0 -
Mouse, and 1 - Pen). So it's more a generic mode for the device than a
multitouch mode.

> 
> Input format on the other hand is a vendor-defined feature that is used
> to switch Wacom devices (both touchscreens and tablets) between sending
> standard HID reports or vendor-defined reports. Devices are supposed to
> start up sending the former, but at least two tablet PCs have had
> hardware that seems to remember being switched to the vendor-defined format.
> 
> If the format is explicitly switched to HID standard, we still should
> probably set the mode as well since I don't think there's anything
> implied about which mode the tablet should use after switching.

OK, then in that regard, it would make more sense to first switch the
device back into the HID mode, and then switch it to multitouch.

> 
> Relatedly, it's probably a good idea to rename 'wacom_set_device_mode'
> or 'wacom_hid_set_device_mode' since they each set different a different
> kind of "mode". It _might_ also be reasonable to rework
> 'wacom_set_device_mode' to be compatible with HID_GENERIC devices
> (rather than introducing 'wacom_hid_set_device_format'), but keeping
> compatibility the explicit devices might be tricky...

OK, how about the following:
we keep wacom_set_device_mode() for switching the proprietary mode, but
add a mode_value and mode_report in wacom_wac. This way, we can detect
those proprietary mode changes in feature_mapping, and still keep the
HID Device Mode bits in place.

I tried to come up with the following, which seems to be working on the
Bamboo PAD, Intuos Pro S, Bamboo 16FG, and Cintiq 13HDT:

---
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 68a5609..90fa4d4 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -322,26 +322,41 @@ static int wacom_hid_set_device_mode(struct hid_device *hdev)
 	return 0;
 }
 
-static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
-		int length, int mode)
+static int wacom_set_device_mode(struct hid_device *hdev,
+				 struct wacom_wac *wacom_wac)
 {
-	unsigned char *rep_data;
+	u8 *rep_data;
+	struct hid_report *r;
+	struct hid_report_enum *re;
+	int length;
 	int error = -ENOMEM, limit = 0;
 
-	rep_data = kzalloc(length, GFP_KERNEL);
+	if (wacom_wac->mode_report < 0)
+		return 0;
+
+	re = &(hdev->report_enum[HID_FEATURE_REPORT]);
+	r = re->report_id_hash[wacom_wac->mode_report];
+	if (!r)
+		return -EINVAL;
+
+	rep_data = hid_alloc_report_buf(r, GFP_KERNEL);
 	if (!rep_data)
-		return error;
+		return -ENOMEM;
+
+	length = hid_report_len(r);
 
 	do {
-		rep_data[0] = report_id;
-		rep_data[1] = mode;
+		rep_data[0] = wacom_wac->mode_report;
+		rep_data[1] = wacom_wac->mode_value;
 
 		error = wacom_set_report(hdev, HID_FEATURE_REPORT, rep_data,
 					 length, 1);
 		if (error >= 0)
 			error = wacom_get_report(hdev, HID_FEATURE_REPORT,
 			                         rep_data, length, 1);
-	} while (error >= 0 && rep_data[1] != mode && limit++ < WAC_MSG_RETRIES);
+	} while (error >= 0 &&
+		 rep_data[1] != wacom_wac->mode_report &&
+		 limit++ < WAC_MSG_RETRIES);
 
 	kfree(rep_data);
 
@@ -411,32 +426,41 @@ static int wacom_bt_query_tablet_data(struct hid_device *hdev, u8 speed,
 static int wacom_query_tablet_data(struct hid_device *hdev,
 		struct wacom_features *features)
 {
+	struct wacom *wacom = hid_get_drvdata(hdev);
+	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+
 	if (hdev->bus == BUS_BLUETOOTH)
 		return wacom_bt_query_tablet_data(hdev, 1, features);
 
-	if (features->type == HID_GENERIC)
-		return wacom_hid_set_device_mode(hdev);
-
-	if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
-		if (features->type > TABLETPC) {
-			/* MT Tablet PC touch */
-			return wacom_set_device_mode(hdev, 3, 4, 4);
-		}
-		else if (features->type == WACOM_24HDT) {
-			return wacom_set_device_mode(hdev, 18, 3, 2);
-		}
-		else if (features->type == WACOM_27QHDT) {
-			return wacom_set_device_mode(hdev, 131, 3, 2);
-		}
-		else if (features->type == BAMBOO_PAD) {
-			return wacom_set_device_mode(hdev, 2, 2, 2);
-		}
-	} else if (features->device_type & WACOM_DEVICETYPE_PEN) {
-		if (features->type <= BAMBOO_PT) {
-			return wacom_set_device_mode(hdev, 2, 2, 2);
+	if (features->type != HID_GENERIC) {
+		if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
+			if (features->type > TABLETPC) {
+				/* MT Tablet PC touch */
+				wacom_wac->mode_report = 3;
+				wacom_wac->mode_value = 4;
+			} else if (features->type == WACOM_24HDT) {
+				wacom_wac->mode_report = 18;
+				wacom_wac->mode_value = 2;
+			} else if (features->type == WACOM_27QHDT) {
+				wacom_wac->mode_report = 131;
+				wacom_wac->mode_value = 2;
+			} else if (features->type == BAMBOO_PAD) {
+				wacom_wac->mode_report = 2;
+				wacom_wac->mode_value = 2;
+			}
+		} else if (features->device_type & WACOM_DEVICETYPE_PEN) {
+			if (features->type <= BAMBOO_PT) {
+				wacom_wac->mode_report = 2;
+				wacom_wac->mode_value = 2;
+			}
 		}
 	}
 
+	wacom_set_device_mode(hdev, wacom_wac);
+
+	if (features->type == HID_GENERIC)
+		return wacom_hid_set_device_mode(hdev);
+
 	return 0;
 }
 
@@ -1817,6 +1841,9 @@ static int wacom_probe(struct hid_device *hdev,
 		goto fail_type;
 	}
 
+	wacom_wac->hid_data.inputmode = -1;
+	wacom_wac->mode_report = -1;
+
 	wacom->usbdev = dev;
 	wacom->intf = intf;
 	mutex_init(&wacom->lock);
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 25baa7f..8f40e45 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -238,6 +238,8 @@ struct wacom_wac {
 	int ps_connected;
 	u8 bt_features;
 	u8 bt_high_speed;
+	int mode_report;
+	int mode_value;
 	struct hid_data hid_data;
 };
  
---
 
This patch infers the length of the feature from the report descriptor,
but I would expect this to be correct on all devices.

Any tests/remarks would be appreciated :)

Cheers,
Benjamin

> 
> Off to work on V2...
> 
> [1]:
> https://msdn.microsoft.com/en-us/library/windows/hardware/ff553739%28v=vs.85%29.aspx
> 
> Jason
> ---
> Now instead of four in the eights place /
> you’ve got three, ‘Cause you added one /
> (That is to say, eight) to the two, /
> But you can’t take seven from three, /
> So you look at the sixty-fours....
> 
> >> +		return err;
> >> +	}
> >>  
> >>  	if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
> >>  		if (features->type > TABLETPC) {
> >> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> >> index 25baa7f..37d5dec 100644
> >> --- a/drivers/hid/wacom_wac.h
> >> +++ b/drivers/hid/wacom_wac.h
> >> @@ -84,6 +84,12 @@
> >>  #define WACOM_DEVICETYPE_WL_MONITOR     0x0008
> >>  
> >>  #define WACOM_VENDORDEFINED_PEN		0xff0d0001
> >> +#define WACOM_G9_PAGE			0xff090000
> >> +#define WACOM_G9_DIGITIZER		(WACOM_G9_PAGE | 0x02)
> >> +#define WACOM_G9_TOUCHSCREEN		(WACOM_G9_PAGE | 0x11)
> >> +#define WACOM_G11_PAGE			0xff110000
> >> +#define WACOM_G11_DIGITIZER		(WACOM_G11_PAGE | 0x02)
> >> +#define WACOM_G11_TOUCHSCREEN		(WACOM_G11_PAGE | 0x11)
> >>  
> >>  #define WACOM_PEN_FIELD(f)	(((f)->logical == HID_DG_STYLUS) || \
> >>  				 ((f)->physical == HID_DG_STYLUS) || \
> >> @@ -193,8 +199,11 @@ struct wacom_shared {
> >>  };
> >>  
> >>  struct hid_data {
> >> -	__s16 inputmode;	/* InputMode HID feature, -1 if non-existent */
> >> -	__s16 inputmode_index;	/* InputMode HID feature index in the report */
> >> +	__s16 inputmode;		/* InputMode HID feature, -1 if non-existent */
> >> +	__s16 inputmode_index;		/* InputMode HID feature index in the report */
> >> +	__s16 inputformat;		/* Vendor-defined "input format" feature, -1 if non-existent */
> >> +	__s16 inputformat_index;	/* Vendor-defined "input format" feature index in the report */
> >> +	__u8  inputformat_value;	/* Value expected by device to switch to HID mode */
> >>  	bool inrange_state;
> >>  	bool invert_state;
> >>  	bool tipswitch;
> >> -- 
> >> 2.7.3
> >>
--
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] HID: wacom: Support switching from vendor-defined input format on G9 and G11

[Jason Gerecke]

On 03/30/2016 02:24 AM, Benjamin Tissoires wrote:
> On Mar 29 2016 or thereabouts, Jason Gerecke wrote:
>> On 03/29/2016 12:05 AM, Benjamin Tissoires wrote:
>>> Hi Jason,
>>>
>>> On Mar 28 2016 or thereabouts, Jason Gerecke wrote:
>>>> A tablet PC booted into Windows may have its pen/touch hardware switched
>>>> into "Wacom mode" similar to what we do with explicitly-supported hardware.
>>>> Some devices appear to maintain this state across reboots, preventing their
>>>> use with the generic HID driver. This patch adds support for the vendor-
>>>> defined "input format" feature report of G9 and G11 chips and has the HID
>>>> codepath always attempt to reset the device back to an appropriate format.
>>>>
>>>> Fixes: https://sourceforge.net/p/linuxwacom/bugs/307/
>>>> Fixes: https://sourceforge.net/p/linuxwacom/bugs/310/
>>>> Fixes: https://github.com/linuxwacom/input-wacom/issues/15
>>>>
>>>> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
>>>> ---
>>>>  drivers/hid/wacom_sys.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++--
>>>>  drivers/hid/wacom_wac.h | 13 ++++++++++--
>>>>  2 files changed, 64 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
>>>> index 68a5609..d7ef7b0 100644
>>>> --- a/drivers/hid/wacom_sys.c
>>>> +++ b/drivers/hid/wacom_sys.c
>>>> @@ -152,6 +152,32 @@ static void wacom_feature_mapping(struct hid_device *hdev,
>>>>  		hid_data->inputmode = field->report->id;
>>>>  		hid_data->inputmode_index = usage->usage_index;
>>>>  		break;
>>>> +
>>>> +	/* The vendor-defined application collections leave most usages
>>>> +	 * as null (0x0000), making them much more difficult to match...
>>>> +	 */
>>>
>>> Not so sure this comments adds anything to the code. The application
>>> usage is well defined, so there is no need to worry I think.
>>>
>>> (Plus the coding style should be:
>>> /*
>>>  * multi lines
>>>  * comment.
>>>  */
>>> )
>>>
>>>> +	case HID_UP_DIGITIZER:
>>>> +		if (field->application == WACOM_G9_DIGITIZER ||
>>>> +		    field->application == WACOM_G11_DIGITIZER) {
>>>> +			if (field->report->id == 0x0B) {
>>>
>>> I'd rather have the test on the report id first, and have only one if
>>> (with id && (app || app). This way, you'll keep the extra tab below.
>>>
>>>> +				hid_data->inputformat = field->report->id;
>>>> +				hid_data->inputformat_index = 0;
>>>
>>> Shouldn't this be usage->usage_index instead? Unless it's defined
>>> several times, which would make the use of usage_index moot.
>>>
>>
>> The field is the entire contents of one of Wacom's infamous "n
>> vendor-defined bytes" reports, so although the usage itself isn't
>> defined multiple times it does have a report count associated with it
>> (with the same net effect).
> 
> Fair enough. But I wonder if we need to keep the inputformat_index field
> given that it will be always 0.
> 
>>
>>>> +				hid_data->inputformat_value = 0; /* 0: HID; 1: Vendor-defined */
>>>
>>> Looks like both usages are requesting this to be set at 0. I think it's
>>> fine to keep it, but I don't know if it's that important to keep then.
>>>
>>>> +			}
>>>> +		}
>>>> +		break;
>>>> +
>>>> +	case WACOM_G9_PAGE:
>>>> +	case WACOM_G11_PAGE:
>>>> +		if (field->application == WACOM_G9_TOUCHSCREEN ||
>>>> +		    field->application == WACOM_G11_TOUCHSCREEN) {
>>>> +			if (field->report->id == 0x03) {
>>>
>>> Same three comments above apply here.
>>>
>>>> +				hid_data->inputformat = field->report->id;
>>>> +				hid_data->inputformat_index = 0;
>>>> +				hid_data->inputformat_value = 0; /* 0: HID; 4: Vendor-defined */
>>>> +			}
>>>> +		}
>>>> +		break;
>>>>  	}
>>>>  }
>>>>  
>>>> @@ -322,6 +348,25 @@ static int wacom_hid_set_device_mode(struct hid_device *hdev)
>>>>  	return 0;
>>>>  }
>>>>  
>>>> +static int wacom_hid_set_device_format(struct hid_device *hdev)
>>>> +{
>>>> +	struct wacom *wacom = hid_get_drvdata(hdev);
>>>> +	struct hid_data *hid_data = &wacom->wacom_wac.hid_data;
>>>> +	struct hid_report *r;
>>>> +	struct hid_report_enum *re;
>>>> +
>>>> +	if (hid_data->inputformat < 0)
>>>> +		return 0;
>>>> +
>>>> +	re = &(hdev->report_enum[HID_FEATURE_REPORT]);
>>>> +	r = re->report_id_hash[hid_data->inputformat];
>>>> +	if (r) {
>>>> +		r->field[0]->value[hid_data->inputformat_index] = hid_data->inputformat_value;
>>>> +		hid_hw_request(hdev, r, HID_REQ_SET_REPORT);
>>>> +	}
>>>> +	return 0;
>>>> +}
>>>> +
>>>>  static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
>>>>  		int length, int mode)
>>>>  {
>>>> @@ -414,8 +459,14 @@ static int wacom_query_tablet_data(struct hid_device *hdev,
>>>>  	if (hdev->bus == BUS_BLUETOOTH)
>>>>  		return wacom_bt_query_tablet_data(hdev, 1, features);
>>>>  
>>>> -	if (features->type == HID_GENERIC)
>>>> -		return wacom_hid_set_device_mode(hdev);
>>>> +	if (features->type == HID_GENERIC) {
>>>> +		int err;
>>>> +		err = wacom_hid_set_device_mode(hdev);
>>>> +		if (err)
>>>> +			return err;
>>>> +		err = wacom_hid_set_device_format(hdev);
>>>
>>> I don't really see the difference between "input mode" and "input
>>> format". Does the device needs both to be set? If not, couldn't we
>>> extend the input mode feature for these usages?
>>>
>>> Cheers,
>>> Benjamin
>>>
>>
>> Device / Input mode is a standard HID feature expected to be present on
>> Windows 7-compatible touch devices which is used to switch a device
>> between sending mouse, single, or multi-touch reports[1]. The
>> 'wacom_hid_set_device_mode' function right now takes care of switching
>> into multi-touch mode provided the hid_data->inputmode field has been set.
> 
> Well, "Device Mode" is also used by MS for Pen devices (values are 0 -
> Mouse, and 1 - Pen). So it's more a generic mode for the device than a
> multitouch mode.
> 
>>
>> Input format on the other hand is a vendor-defined feature that is used
>> to switch Wacom devices (both touchscreens and tablets) between sending
>> standard HID reports or vendor-defined reports. Devices are supposed to
>> start up sending the former, but at least two tablet PCs have had
>> hardware that seems to remember being switched to the vendor-defined format.
>>
>> If the format is explicitly switched to HID standard, we still should
>> probably set the mode as well since I don't think there's anything
>> implied about which mode the tablet should use after switching.
> 
> OK, then in that regard, it would make more sense to first switch the
> device back into the HID mode, and then switch it to multitouch.
> 
>>
>> Relatedly, it's probably a good idea to rename 'wacom_set_device_mode'
>> or 'wacom_hid_set_device_mode' since they each set different a different
>> kind of "mode". It _might_ also be reasonable to rework
>> 'wacom_set_device_mode' to be compatible with HID_GENERIC devices
>> (rather than introducing 'wacom_hid_set_device_format'), but keeping
>> compatibility the explicit devices might be tricky...
> 
> OK, how about the following:
> we keep wacom_set_device_mode() for switching the proprietary mode, but
> add a mode_value and mode_report in wacom_wac. This way, we can detect
> those proprietary mode changes in feature_mapping, and still keep the
> HID Device Mode bits in place.
> 
> I tried to come up with the following, which seems to be working on the
> Bamboo PAD, Intuos Pro S, Bamboo 16FG, and Cintiq 13HDT:
> 
> ---
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 68a5609..90fa4d4 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -322,26 +322,41 @@ static int wacom_hid_set_device_mode(struct hid_device *hdev)
>  	return 0;
>  }
>  
> -static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
> -		int length, int mode)
> +static int wacom_set_device_mode(struct hid_device *hdev,
> +				 struct wacom_wac *wacom_wac)
>  {
> -	unsigned char *rep_data;
> +	u8 *rep_data;
> +	struct hid_report *r;
> +	struct hid_report_enum *re;
> +	int length;
>  	int error = -ENOMEM, limit = 0;
>  
> -	rep_data = kzalloc(length, GFP_KERNEL);
> +	if (wacom_wac->mode_report < 0)
> +		return 0;
> +
> +	re = &(hdev->report_enum[HID_FEATURE_REPORT]);
> +	r = re->report_id_hash[wacom_wac->mode_report];
> +	if (!r)
> +		return -EINVAL;
> +
> +	rep_data = hid_alloc_report_buf(r, GFP_KERNEL);
>  	if (!rep_data)
> -		return error;
> +		return -ENOMEM;
> +
> +	length = hid_report_len(r);
>  
>  	do {
> -		rep_data[0] = report_id;
> -		rep_data[1] = mode;
> +		rep_data[0] = wacom_wac->mode_report;
> +		rep_data[1] = wacom_wac->mode_value;
>  
>  		error = wacom_set_report(hdev, HID_FEATURE_REPORT, rep_data,
>  					 length, 1);
>  		if (error >= 0)
>  			error = wacom_get_report(hdev, HID_FEATURE_REPORT,
>  			                         rep_data, length, 1);
> -	} while (error >= 0 && rep_data[1] != mode && limit++ < WAC_MSG_RETRIES);
> +	} while (error >= 0 &&
> +		 rep_data[1] != wacom_wac->mode_report &&
> +		 limit++ < WAC_MSG_RETRIES);
>  
>  	kfree(rep_data);
>  
> @@ -411,32 +426,41 @@ static int wacom_bt_query_tablet_data(struct hid_device *hdev, u8 speed,
>  static int wacom_query_tablet_data(struct hid_device *hdev,
>  		struct wacom_features *features)
>  {
> +	struct wacom *wacom = hid_get_drvdata(hdev);
> +	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
> +
>  	if (hdev->bus == BUS_BLUETOOTH)
>  		return wacom_bt_query_tablet_data(hdev, 1, features);
>  
> -	if (features->type == HID_GENERIC)
> -		return wacom_hid_set_device_mode(hdev);
> -
> -	if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
> -		if (features->type > TABLETPC) {
> -			/* MT Tablet PC touch */
> -			return wacom_set_device_mode(hdev, 3, 4, 4);
> -		}
> -		else if (features->type == WACOM_24HDT) {
> -			return wacom_set_device_mode(hdev, 18, 3, 2);
> -		}
> -		else if (features->type == WACOM_27QHDT) {
> -			return wacom_set_device_mode(hdev, 131, 3, 2);
> -		}
> -		else if (features->type == BAMBOO_PAD) {
> -			return wacom_set_device_mode(hdev, 2, 2, 2);
> -		}
> -	} else if (features->device_type & WACOM_DEVICETYPE_PEN) {
> -		if (features->type <= BAMBOO_PT) {
> -			return wacom_set_device_mode(hdev, 2, 2, 2);
> +	if (features->type != HID_GENERIC) {
> +		if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
> +			if (features->type > TABLETPC) {
> +				/* MT Tablet PC touch */
> +				wacom_wac->mode_report = 3;
> +				wacom_wac->mode_value = 4;
> +			} else if (features->type == WACOM_24HDT) {
> +				wacom_wac->mode_report = 18;
> +				wacom_wac->mode_value = 2;
> +			} else if (features->type == WACOM_27QHDT) {
> +				wacom_wac->mode_report = 131;
> +				wacom_wac->mode_value = 2;
> +			} else if (features->type == BAMBOO_PAD) {
> +				wacom_wac->mode_report = 2;
> +				wacom_wac->mode_value = 2;
> +			}
> +		} else if (features->device_type & WACOM_DEVICETYPE_PEN) {
> +			if (features->type <= BAMBOO_PT) {
> +				wacom_wac->mode_report = 2;
> +				wacom_wac->mode_value = 2;
> +			}
>  		}
>  	}
>  
> +	wacom_set_device_mode(hdev, wacom_wac);
> +
> +	if (features->type == HID_GENERIC)
> +		return wacom_hid_set_device_mode(hdev);
> +
>  	return 0;
>  }
>  
> @@ -1817,6 +1841,9 @@ static int wacom_probe(struct hid_device *hdev,
>  		goto fail_type;
>  	}
>  
> +	wacom_wac->hid_data.inputmode = -1;

I noticed this was missing the other day but then totally forgot to fix
it. Thanks for the reminder :)

> +	wacom_wac->mode_report = -1;
> +
>  	wacom->usbdev = dev;
>  	wacom->intf = intf;
>  	mutex_init(&wacom->lock);
> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> index 25baa7f..8f40e45 100644
> --- a/drivers/hid/wacom_wac.h
> +++ b/drivers/hid/wacom_wac.h
> @@ -238,6 +238,8 @@ struct wacom_wac {
>  	int ps_connected;
>  	u8 bt_features;
>  	u8 bt_high_speed;
> +	int mode_report;
> +	int mode_value;
>  	struct hid_data hid_data;
>  };
>   
> ---
>  
> This patch infers the length of the feature from the report descriptor,
> but I would expect this to be correct on all devices.
> 
> Any tests/remarks would be appreciated :)
> 
> Cheers,
> Benjamin
> 

I think I like the look of this :) I'll do a bit more checking on my end
and get back to you shortly.

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....

>>
>> Off to work on V2...
>>
>> [1]:
>> https://msdn.microsoft.com/en-us/library/windows/hardware/ff553739%28v=vs.85%29.aspx
>>
>> Jason
>> ---
>> Now instead of four in the eights place /
>> you’ve got three, ‘Cause you added one /
>> (That is to say, eight) to the two, /
>> But you can’t take seven from three, /
>> So you look at the sixty-fours....
>>
>>>> +		return err;
>>>> +	}
>>>>  
>>>>  	if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
>>>>  		if (features->type > TABLETPC) {
>>>> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
>>>> index 25baa7f..37d5dec 100644
>>>> --- a/drivers/hid/wacom_wac.h
>>>> +++ b/drivers/hid/wacom_wac.h
>>>> @@ -84,6 +84,12 @@
>>>>  #define WACOM_DEVICETYPE_WL_MONITOR     0x0008
>>>>  
>>>>  #define WACOM_VENDORDEFINED_PEN		0xff0d0001
>>>> +#define WACOM_G9_PAGE			0xff090000
>>>> +#define WACOM_G9_DIGITIZER		(WACOM_G9_PAGE | 0x02)
>>>> +#define WACOM_G9_TOUCHSCREEN		(WACOM_G9_PAGE | 0x11)
>>>> +#define WACOM_G11_PAGE			0xff110000
>>>> +#define WACOM_G11_DIGITIZER		(WACOM_G11_PAGE | 0x02)
>>>> +#define WACOM_G11_TOUCHSCREEN		(WACOM_G11_PAGE | 0x11)
>>>>  
>>>>  #define WACOM_PEN_FIELD(f)	(((f)->logical == HID_DG_STYLUS) || \
>>>>  				 ((f)->physical == HID_DG_STYLUS) || \
>>>> @@ -193,8 +199,11 @@ struct wacom_shared {
>>>>  };
>>>>  
>>>>  struct hid_data {
>>>> -	__s16 inputmode;	/* InputMode HID feature, -1 if non-existent */
>>>> -	__s16 inputmode_index;	/* InputMode HID feature index in the report */
>>>> +	__s16 inputmode;		/* InputMode HID feature, -1 if non-existent */
>>>> +	__s16 inputmode_index;		/* InputMode HID feature index in the report */
>>>> +	__s16 inputformat;		/* Vendor-defined "input format" feature, -1 if non-existent */
>>>> +	__s16 inputformat_index;	/* Vendor-defined "input format" feature index in the report */
>>>> +	__u8  inputformat_value;	/* Value expected by device to switch to HID mode */
>>>>  	bool inrange_state;
>>>>  	bool invert_state;
>>>>  	bool tipswitch;
>>>> -- 
>>>> 2.7.3
>>>>
--
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 1/2] HID: wacom: Initialize hid_data.inputmode to -1

[Jason Gerecke]

Commit 5ae6e89 introduced hid_data.inputmode with a comment that it
would have the value -1 if undefined, but then forgot to actually
perform the initialization. Although this doesn't appear to have
caused any problems in practice, it should still be remedied.

Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
Changes from v1:
 * Introduce this patch

 drivers/hid/wacom_sys.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 68a5609..b338bbf 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1817,6 +1817,8 @@ static int wacom_probe(struct hid_device *hdev,
 		goto fail_type;
 	}
 
+	wacom_wac->hid_data.inputmode = -1;
+
 	wacom->usbdev = dev;
 	wacom->intf = intf;
 	mutex_init(&wacom->lock);
-- 
2.7.3

[PATCH v2 2/2] HID: wacom: Support switching from vendor-defined device mode on G9 and G11

[Jason Gerecke]

A tablet PC booted into Windows may have its pen/touch hardware switched
into "Wacom mode" similar to what we do with explicitly-supported hardware.
Some devices appear to maintain this state across reboots, preventing their
use with the generic HID driver. This patch adds support for detecting the
presence of the mode switch feature report used by devices based on the G9
and G11 chips and has the HID codepath always attempt to reset the device
back to sending standard HID reports.

Fixes: https://sourceforge.net/p/linuxwacom/bugs/307/
Fixes: https://sourceforge.net/p/linuxwacom/bugs/310/
Fixes: https://github.com/linuxwacom/input-wacom/issues/15

Co-authored-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
Changes from v1:
 * Incorporate Benjamin's feedback & suggested wacom_set_device_mode code

 drivers/hid/wacom_sys.c | 100 ++++++++++++++++++++++++++++++++++--------------
 drivers/hid/wacom_wac.h |   8 ++++
 2 files changed, 80 insertions(+), 28 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index b338bbf..ccf1883 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -152,6 +152,25 @@ static void wacom_feature_mapping(struct hid_device *hdev,
 		hid_data->inputmode = field->report->id;
 		hid_data->inputmode_index = usage->usage_index;
 		break;
+
+	case HID_UP_DIGITIZER:
+		if (field->report->id == 0x0B &&
+		    (field->application == WACOM_G9_DIGITIZER ||
+		     field->application == WACOM_G11_DIGITIZER)) {
+			wacom->wacom_wac.mode_report = field->report->id;
+			wacom->wacom_wac.mode_value = 0;
+		}
+		break;
+
+	case WACOM_G9_PAGE:
+	case WACOM_G11_PAGE:
+		if (field->report->id == 0x03 &&
+		    (field->application == WACOM_G9_TOUCHSCREEN ||
+		     field->application == WACOM_G11_TOUCHSCREEN)) {
+			wacom->wacom_wac.mode_report = field->report->id;
+			wacom->wacom_wac.mode_value = 0;
+		}
+		break;
 	}
 }
 
@@ -322,26 +341,41 @@ static int wacom_hid_set_device_mode(struct hid_device *hdev)
 	return 0;
 }
 
-static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
-		int length, int mode)
+static int wacom_set_device_mode(struct hid_device *hdev,
+				 struct wacom_wac *wacom_wac)
 {
-	unsigned char *rep_data;
+	u8 *rep_data;
+	struct hid_report *r;
+	struct hid_report_enum *re;
+	int length;
 	int error = -ENOMEM, limit = 0;
 
-	rep_data = kzalloc(length, GFP_KERNEL);
+	if (wacom_wac->mode_report < 0)
+		return 0;
+
+	re = &(hdev->report_enum[HID_FEATURE_REPORT]);
+	r = re->report_id_hash[wacom_wac->mode_report];
+	if (!r)
+		return -EINVAL;
+
+	rep_data = hid_alloc_report_buf(r, GFP_KERNEL);
 	if (!rep_data)
-		return error;
+		return -ENOMEM;
+
+	length = hid_report_len(r);
 
 	do {
-		rep_data[0] = report_id;
-		rep_data[1] = mode;
+		rep_data[0] = wacom_wac->mode_report;
+		rep_data[1] = wacom_wac->mode_value;
 
 		error = wacom_set_report(hdev, HID_FEATURE_REPORT, rep_data,
 					 length, 1);
 		if (error >= 0)
 			error = wacom_get_report(hdev, HID_FEATURE_REPORT,
 			                         rep_data, length, 1);
-	} while (error >= 0 && rep_data[1] != mode && limit++ < WAC_MSG_RETRIES);
+	} while (error >= 0 &&
+		 rep_data[1] != wacom_wac->mode_report &&
+		 limit++ < WAC_MSG_RETRIES);
 
 	kfree(rep_data);
 
@@ -411,32 +445,41 @@ static int wacom_bt_query_tablet_data(struct hid_device *hdev, u8 speed,
 static int wacom_query_tablet_data(struct hid_device *hdev,
 		struct wacom_features *features)
 {
+	struct wacom *wacom = hid_get_drvdata(hdev);
+	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+
 	if (hdev->bus == BUS_BLUETOOTH)
 		return wacom_bt_query_tablet_data(hdev, 1, features);
 
-	if (features->type == HID_GENERIC)
-		return wacom_hid_set_device_mode(hdev);
-
-	if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
-		if (features->type > TABLETPC) {
-			/* MT Tablet PC touch */
-			return wacom_set_device_mode(hdev, 3, 4, 4);
-		}
-		else if (features->type == WACOM_24HDT) {
-			return wacom_set_device_mode(hdev, 18, 3, 2);
-		}
-		else if (features->type == WACOM_27QHDT) {
-			return wacom_set_device_mode(hdev, 131, 3, 2);
-		}
-		else if (features->type == BAMBOO_PAD) {
-			return wacom_set_device_mode(hdev, 2, 2, 2);
-		}
-	} else if (features->device_type & WACOM_DEVICETYPE_PEN) {
-		if (features->type <= BAMBOO_PT) {
-			return wacom_set_device_mode(hdev, 2, 2, 2);
+	if (features->type != HID_GENERIC) {
+		if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
+			if (features->type > TABLETPC) {
+				/* MT Tablet PC touch */
+				wacom_wac->mode_report = 3;
+				wacom_wac->mode_value = 4;
+			} else if (features->type == WACOM_24HDT) {
+				wacom_wac->mode_report = 18;
+				wacom_wac->mode_value = 2;
+			} else if (features->type == WACOM_27QHDT) {
+				wacom_wac->mode_report = 131;
+				wacom_wac->mode_value = 2;
+			} else if (features->type == BAMBOO_PAD) {
+				wacom_wac->mode_report = 2;
+				wacom_wac->mode_value = 2;
+			}
+		} else if (features->device_type & WACOM_DEVICETYPE_PEN) {
+			if (features->type <= BAMBOO_PT) {
+				wacom_wac->mode_report = 2;
+				wacom_wac->mode_value = 2;
+			}
 		}
 	}
 
+	wacom_set_device_mode(hdev, wacom_wac);
+
+	if (features->type == HID_GENERIC)
+		return wacom_hid_set_device_mode(hdev);
+
 	return 0;
 }
 
@@ -1818,6 +1861,7 @@ static int wacom_probe(struct hid_device *hdev,
 	}
 
 	wacom_wac->hid_data.inputmode = -1;
+	wacom_wac->mode_report = -1;
 
 	wacom->usbdev = dev;
 	wacom->intf = intf;
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 25baa7f..e2084d9 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -84,6 +84,12 @@
 #define WACOM_DEVICETYPE_WL_MONITOR     0x0008
 
 #define WACOM_VENDORDEFINED_PEN		0xff0d0001
+#define WACOM_G9_PAGE			0xff090000
+#define WACOM_G9_DIGITIZER		(WACOM_G9_PAGE | 0x02)
+#define WACOM_G9_TOUCHSCREEN		(WACOM_G9_PAGE | 0x11)
+#define WACOM_G11_PAGE			0xff110000
+#define WACOM_G11_DIGITIZER		(WACOM_G11_PAGE | 0x02)
+#define WACOM_G11_TOUCHSCREEN		(WACOM_G11_PAGE | 0x11)
 
 #define WACOM_PEN_FIELD(f)	(((f)->logical == HID_DG_STYLUS) || \
 				 ((f)->physical == HID_DG_STYLUS) || \
@@ -238,6 +244,8 @@ struct wacom_wac {
 	int ps_connected;
 	u8 bt_features;
 	u8 bt_high_speed;
+	int mode_report;
+	int mode_value;
 	struct hid_data hid_data;
 };
 
-- 
2.7.3

Re: [PATCH v2 2/2] HID: wacom: Support switching from vendor-defined device mode on G9 and G11

[Benjamin Tissoires]

On Apr 04 2016 or thereabouts, Jason Gerecke wrote:
> A tablet PC booted into Windows may have its pen/touch hardware switched
> into "Wacom mode" similar to what we do with explicitly-supported hardware.
> Some devices appear to maintain this state across reboots, preventing their
> use with the generic HID driver. This patch adds support for detecting the
> presence of the mode switch feature report used by devices based on the G9
> and G11 chips and has the HID codepath always attempt to reset the device
> back to sending standard HID reports.
> 
> Fixes: https://sourceforge.net/p/linuxwacom/bugs/307/
> Fixes: https://sourceforge.net/p/linuxwacom/bugs/310/
> Fixes: https://github.com/linuxwacom/input-wacom/issues/15
> 
> Co-authored-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> ---

Not sure how much this matters now, but for the series:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

> Changes from v1:
>  * Incorporate Benjamin's feedback & suggested wacom_set_device_mode code
> 
>  drivers/hid/wacom_sys.c | 100 ++++++++++++++++++++++++++++++++++--------------
>  drivers/hid/wacom_wac.h |   8 ++++
>  2 files changed, 80 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index b338bbf..ccf1883 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -152,6 +152,25 @@ static void wacom_feature_mapping(struct hid_device *hdev,
>  		hid_data->inputmode = field->report->id;
>  		hid_data->inputmode_index = usage->usage_index;
>  		break;
> +
> +	case HID_UP_DIGITIZER:
> +		if (field->report->id == 0x0B &&
> +		    (field->application == WACOM_G9_DIGITIZER ||
> +		     field->application == WACOM_G11_DIGITIZER)) {
> +			wacom->wacom_wac.mode_report = field->report->id;
> +			wacom->wacom_wac.mode_value = 0;
> +		}
> +		break;
> +
> +	case WACOM_G9_PAGE:
> +	case WACOM_G11_PAGE:
> +		if (field->report->id == 0x03 &&
> +		    (field->application == WACOM_G9_TOUCHSCREEN ||
> +		     field->application == WACOM_G11_TOUCHSCREEN)) {
> +			wacom->wacom_wac.mode_report = field->report->id;
> +			wacom->wacom_wac.mode_value = 0;
> +		}
> +		break;
>  	}
>  }
>  
> @@ -322,26 +341,41 @@ static int wacom_hid_set_device_mode(struct hid_device *hdev)
>  	return 0;
>  }
>  
> -static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
> -		int length, int mode)
> +static int wacom_set_device_mode(struct hid_device *hdev,
> +				 struct wacom_wac *wacom_wac)
>  {
> -	unsigned char *rep_data;
> +	u8 *rep_data;
> +	struct hid_report *r;
> +	struct hid_report_enum *re;
> +	int length;
>  	int error = -ENOMEM, limit = 0;
>  
> -	rep_data = kzalloc(length, GFP_KERNEL);
> +	if (wacom_wac->mode_report < 0)
> +		return 0;
> +
> +	re = &(hdev->report_enum[HID_FEATURE_REPORT]);
> +	r = re->report_id_hash[wacom_wac->mode_report];
> +	if (!r)
> +		return -EINVAL;
> +
> +	rep_data = hid_alloc_report_buf(r, GFP_KERNEL);
>  	if (!rep_data)
> -		return error;
> +		return -ENOMEM;
> +
> +	length = hid_report_len(r);
>  
>  	do {
> -		rep_data[0] = report_id;
> -		rep_data[1] = mode;
> +		rep_data[0] = wacom_wac->mode_report;
> +		rep_data[1] = wacom_wac->mode_value;
>  
>  		error = wacom_set_report(hdev, HID_FEATURE_REPORT, rep_data,
>  					 length, 1);
>  		if (error >= 0)
>  			error = wacom_get_report(hdev, HID_FEATURE_REPORT,
>  			                         rep_data, length, 1);
> -	} while (error >= 0 && rep_data[1] != mode && limit++ < WAC_MSG_RETRIES);
> +	} while (error >= 0 &&
> +		 rep_data[1] != wacom_wac->mode_report &&
> +		 limit++ < WAC_MSG_RETRIES);
>  
>  	kfree(rep_data);
>  
> @@ -411,32 +445,41 @@ static int wacom_bt_query_tablet_data(struct hid_device *hdev, u8 speed,
>  static int wacom_query_tablet_data(struct hid_device *hdev,
>  		struct wacom_features *features)
>  {
> +	struct wacom *wacom = hid_get_drvdata(hdev);
> +	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
> +
>  	if (hdev->bus == BUS_BLUETOOTH)
>  		return wacom_bt_query_tablet_data(hdev, 1, features);
>  
> -	if (features->type == HID_GENERIC)
> -		return wacom_hid_set_device_mode(hdev);
> -
> -	if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
> -		if (features->type > TABLETPC) {
> -			/* MT Tablet PC touch */
> -			return wacom_set_device_mode(hdev, 3, 4, 4);
> -		}
> -		else if (features->type == WACOM_24HDT) {
> -			return wacom_set_device_mode(hdev, 18, 3, 2);
> -		}
> -		else if (features->type == WACOM_27QHDT) {
> -			return wacom_set_device_mode(hdev, 131, 3, 2);
> -		}
> -		else if (features->type == BAMBOO_PAD) {
> -			return wacom_set_device_mode(hdev, 2, 2, 2);
> -		}
> -	} else if (features->device_type & WACOM_DEVICETYPE_PEN) {
> -		if (features->type <= BAMBOO_PT) {
> -			return wacom_set_device_mode(hdev, 2, 2, 2);
> +	if (features->type != HID_GENERIC) {
> +		if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
> +			if (features->type > TABLETPC) {
> +				/* MT Tablet PC touch */
> +				wacom_wac->mode_report = 3;
> +				wacom_wac->mode_value = 4;
> +			} else if (features->type == WACOM_24HDT) {
> +				wacom_wac->mode_report = 18;
> +				wacom_wac->mode_value = 2;
> +			} else if (features->type == WACOM_27QHDT) {
> +				wacom_wac->mode_report = 131;
> +				wacom_wac->mode_value = 2;
> +			} else if (features->type == BAMBOO_PAD) {
> +				wacom_wac->mode_report = 2;
> +				wacom_wac->mode_value = 2;
> +			}
> +		} else if (features->device_type & WACOM_DEVICETYPE_PEN) {
> +			if (features->type <= BAMBOO_PT) {
> +				wacom_wac->mode_report = 2;
> +				wacom_wac->mode_value = 2;
> +			}
>  		}
>  	}
>  
> +	wacom_set_device_mode(hdev, wacom_wac);
> +
> +	if (features->type == HID_GENERIC)
> +		return wacom_hid_set_device_mode(hdev);
> +
>  	return 0;
>  }
>  
> @@ -1818,6 +1861,7 @@ static int wacom_probe(struct hid_device *hdev,
>  	}
>  
>  	wacom_wac->hid_data.inputmode = -1;
> +	wacom_wac->mode_report = -1;
>  
>  	wacom->usbdev = dev;
>  	wacom->intf = intf;
> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> index 25baa7f..e2084d9 100644
> --- a/drivers/hid/wacom_wac.h
> +++ b/drivers/hid/wacom_wac.h
> @@ -84,6 +84,12 @@
>  #define WACOM_DEVICETYPE_WL_MONITOR     0x0008
>  
>  #define WACOM_VENDORDEFINED_PEN		0xff0d0001
> +#define WACOM_G9_PAGE			0xff090000
> +#define WACOM_G9_DIGITIZER		(WACOM_G9_PAGE | 0x02)
> +#define WACOM_G9_TOUCHSCREEN		(WACOM_G9_PAGE | 0x11)
> +#define WACOM_G11_PAGE			0xff110000
> +#define WACOM_G11_DIGITIZER		(WACOM_G11_PAGE | 0x02)
> +#define WACOM_G11_TOUCHSCREEN		(WACOM_G11_PAGE | 0x11)
>  
>  #define WACOM_PEN_FIELD(f)	(((f)->logical == HID_DG_STYLUS) || \
>  				 ((f)->physical == HID_DG_STYLUS) || \
> @@ -238,6 +244,8 @@ struct wacom_wac {
>  	int ps_connected;
>  	u8 bt_features;
>  	u8 bt_high_speed;
> +	int mode_report;
> +	int mode_value;
>  	struct hid_data hid_data;
>  };
>  
> -- 
> 2.7.3
> 

Re: [PATCH v2 2/2] HID: wacom: Support switching from vendor-defined device mode on G9 and G11

[Jiri Kosina]

On Mon, 4 Apr 2016, Benjamin Tissoires wrote:

> On Apr 04 2016 or thereabouts, Jason Gerecke wrote:
> > A tablet PC booted into Windows may have its pen/touch hardware switched
> > into "Wacom mode" similar to what we do with explicitly-supported hardware.
> > Some devices appear to maintain this state across reboots, preventing their
> > use with the generic HID driver. This patch adds support for detecting the
> > presence of the mode switch feature report used by devices based on the G9
> > and G11 chips and has the HID codepath always attempt to reset the device
> > back to sending standard HID reports.
> > 
> > Fixes: https://sourceforge.net/p/linuxwacom/bugs/307/
> > Fixes: https://sourceforge.net/p/linuxwacom/bugs/310/
> > Fixes: https://github.com/linuxwacom/input-wacom/issues/15
> > 
> > Co-authored-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> > ---
> 
> Not sure how much this matters now, but for the series:
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Both applied to for-4.6/upstream-fixes, thanks.

-- 
Jiri Kosina
SUSE Labs