[PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type

[PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type

[Jason Gerecke]

The naming logic currently assumes that all devices will be a pen, finger,
or pad. Though this has historically been the case, the new HID_GENERIC
catch-all may cause us to probe devices with Wacom's 056A VID which aren't
any of these types (e.g. the "Cintiq 24HDT Monitor Control"). This patch
updates the logic so that no suffix will be added to the device name if
the device type is unknown.

Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
 drivers/hid/wacom_sys.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 9c57ac0..222baf5 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1440,12 +1440,15 @@ static void wacom_update_name(struct wacom *wacom)
 	snprintf(wacom_wac->pad_name, sizeof(wacom_wac->pad_name),
 		"%s Pad", wacom_wac->name);
 
-	if (features->device_type != BTN_TOOL_FINGER)
+	if (features->device_type == BTN_TOOL_PEN) {
 		strlcat(wacom_wac->name, " Pen", WACOM_NAME_MAX);
-	else if (features->touch_max)
-		strlcat(wacom_wac->name, " Finger", WACOM_NAME_MAX);
-	else
-		strlcat(wacom_wac->name, " Pad", WACOM_NAME_MAX);
+	}
+	else if (features->device_type == BTN_TOOL_FINGER) {
+		if (features->touch_max)
+			strlcat(wacom_wac->name, " Finger", WACOM_NAME_MAX);
+		else
+			strlcat(wacom_wac->name, " Pad", WACOM_NAME_MAX);
+	}
 }
 
 static int wacom_probe(struct hid_device *hdev,
-- 
2.3.5

[PATCH 2/3] HID: wacom: Discover device_type from HID descriptor for all devices

[Jason Gerecke]

Currently, we assume a device_type of BTN_TOOL_PEN before scanning the
HID descriptor and then change the device_type if what we discover
proves that assumption wrong. This way of doing things makes it more
difficult to figure out if a device (particularly a HID_GENERIC device)
actually does tablet/touch input or is something completley different.

This patch leaves device_type at its initial value of 0 and then calls
'wacom_parse_hid' for every device (not just those that have touch).
As we map the usages, we can set the device_type as before. After we're
finished, we can then check if the value is still zero and do whatever
is most appropriate.

Detecting the pen can be a little tricky on most Wacom devices because
the descriptors describe opaque blobs. Fortunately, older Wacom tablets
have the HID_DG_DIGITIZER usage on the pen's application collection and
newer tablets seem to have a similar vendor-defined usage that we can
trigger on.

Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
 drivers/hid/wacom_sys.c | 23 +++++++++++++----------
 drivers/hid/wacom_wac.c |  8 +++++---
 drivers/hid/wacom_wac.h |  6 +++++-
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 222baf5..157aa7a 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -181,7 +181,11 @@ static void wacom_usage_mapping(struct hid_device *hdev,
 	* X/Y values and some cases of invalid Digitizer X/Y
 	* values commonly reported.
 	*/
-	if (!pen && !finger)
+	if (pen)
+		features->device_type = BTN_TOOL_PEN;
+	else if (finger)
+		features->device_type = BTN_TOOL_FINGER;
+	else
 		return;
 
 	/*
@@ -198,14 +202,11 @@ static void wacom_usage_mapping(struct hid_device *hdev,
 	case HID_GD_X:
 		features->x_max = field->logical_maximum;
 		if (finger) {
-			features->device_type = BTN_TOOL_FINGER;
 			features->x_phy = field->physical_maximum;
 			if (features->type != BAMBOO_PT) {
 				features->unit = field->unit;
 				features->unitExpo = field->unit_exponent;
 			}
-		} else {
-			features->device_type = BTN_TOOL_PEN;
 		}
 		break;
 	case HID_GD_Y:
@@ -425,7 +426,6 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
 	struct usb_interface *intf = wacom->intf;
 
 	/* default features */
-	features->device_type = BTN_TOOL_PEN;
 	features->x_fuzz = 4;
 	features->y_fuzz = 4;
 	features->pressure_fuzz = 0;
@@ -446,10 +446,6 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
 		}
 	}
 
-	/* only devices that support touch need to retrieve the info */
-	if (features->type < BAMBOO_PT)
-		return;
-
 	wacom_parse_hid(hdev, features);
 }
 
@@ -1529,8 +1525,15 @@ static int wacom_probe(struct hid_device *hdev,
 
 	/* Retrieve the physical and logical size for touch devices */
 	wacom_retrieve_hid_descriptor(hdev, features);
-
 	wacom_setup_device_quirks(wacom);
+
+	if (!features->device_type && features->type != WIRELESS) {
+		dev_warn(&hdev->dev, "Unknown device_type for '%s'. %s.",
+			 hdev->name, "Assuming pen");
+
+		features->device_type = BTN_TOOL_PEN;
+	}
+
 	wacom_calculate_res(features);
 
 	wacom_update_name(wacom);
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index dff99ff..a52fc25 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -2186,13 +2186,15 @@ void wacom_setup_device_quirks(struct wacom *wacom)
 
 			features->x_max = 4096;
 			features->y_max = 4096;
-		} else {
-			features->device_type = BTN_TOOL_PEN;
 		}
 	}
 
 	/*
-	 * Same thing for Bamboo PAD
+	 * Raw Wacom-mode pen and touch events both come from interface
+	 * 0, whose HID descriptor has an application usage of 0xFF0D
+	 * (i.e., WACOM_VENDORDEFINED_PEN). We route pen packets back
+	 * out through the HID_GENERIC device created for interface 1,
+	 * so rewrite this one to be of type BTN_TOOL_FINGER.
 	 */
 	if (features->type == BAMBOO_PAD)
 		features->device_type = BTN_TOOL_FINGER;
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index f5a5f68..9a5ee62 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -72,10 +72,14 @@
 #define WACOM_QUIRK_MONITOR		0x0004
 #define WACOM_QUIRK_BATTERY		0x0008
 
+#define WACOM_VENDORDEFINED_PEN		0xff0d0001
+
 #define WACOM_PEN_FIELD(f)	(((f)->logical == HID_DG_STYLUS) || \
 				 ((f)->physical == HID_DG_STYLUS) || \
 				 ((f)->physical == HID_DG_PEN) || \
-				 ((f)->application == HID_DG_PEN))
+				 ((f)->application == HID_DG_PEN) || \
+				 ((f)->application == HID_DG_DIGITIZER) || \
+				 ((f)->application == WACOM_VENDORDEFINED_PEN))
 #define WACOM_FINGER_FIELD(f)	(((f)->logical == HID_DG_FINGER) || \
 				 ((f)->physical == HID_DG_FINGER) || \
 				 ((f)->application == HID_DG_TOUCHSCREEN))
-- 
2.3.5

[PATCH 3/3] HID: wacom: Fail probe if HID_GENERIC device has unknown device_type

[Jason Gerecke]

The last patch was careful to maintain backwards-compatible behavior
by forcing device_type to BTN_TOOL_PEN (and printing a warning) if it
were still uninitialized after scanning the HID descriptor and applying
quirks. We should be more strict with HID_GENERIC devices, however,
since there is no a priori guarantee that it is a tablet or touchpad.
If the device_type is still uninitialized for a HID_GENERIC device then
we assume that it isn't something the driver can work with and so fail
the probe.

Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
 drivers/hid/wacom_sys.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 157aa7a..7abf52c 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1528,8 +1528,14 @@ static int wacom_probe(struct hid_device *hdev,
 	wacom_setup_device_quirks(wacom);
 
 	if (!features->device_type && features->type != WIRELESS) {
+		error = features->type == HID_GENERIC ? -ENODEV : 0;
+
 		dev_warn(&hdev->dev, "Unknown device_type for '%s'. %s.",
-			 hdev->name, "Assuming pen");
+			 hdev->name,
+			 error ? "Ignoring" : "Assuming pen");
+
+		if (error)
+			goto fail_shared_data;
 
 		features->device_type = BTN_TOOL_PEN;
 	}
-- 
2.3.5

Re: [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type

[Ping Cheng]

On Thu, Apr 30, 2015 at 5:51 PM, Jason Gerecke <killertofu@gmail.com> wrote:
> The naming logic currently assumes that all devices will be a pen, finger,
> or pad. Though this has historically been the case, the new HID_GENERIC
> catch-all may cause us to probe devices with Wacom's 056A VID which aren't
> any of these types (e.g. the "Cintiq 24HDT Monitor Control"). This patch
> updates the logic so that no suffix will be added to the device name if
> the device type is unknown.
>
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>

Reviewed-by: Ping Cheng <pingc@wacom.com> for the whole set.

Cheers,

Ping

> ---
>  drivers/hid/wacom_sys.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 9c57ac0..222baf5 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -1440,12 +1440,15 @@ static void wacom_update_name(struct wacom *wacom)
>         snprintf(wacom_wac->pad_name, sizeof(wacom_wac->pad_name),
>                 "%s Pad", wacom_wac->name);
>
> -       if (features->device_type != BTN_TOOL_FINGER)
> +       if (features->device_type == BTN_TOOL_PEN) {
>                 strlcat(wacom_wac->name, " Pen", WACOM_NAME_MAX);
> -       else if (features->touch_max)
> -               strlcat(wacom_wac->name, " Finger", WACOM_NAME_MAX);
> -       else
> -               strlcat(wacom_wac->name, " Pad", WACOM_NAME_MAX);
> +       }
> +       else if (features->device_type == BTN_TOOL_FINGER) {
> +               if (features->touch_max)
> +                       strlcat(wacom_wac->name, " Finger", WACOM_NAME_MAX);
> +               else
> +                       strlcat(wacom_wac->name, " Pad", WACOM_NAME_MAX);
> +       }
>  }
>
>  static int wacom_probe(struct hid_device *hdev,
> --
> 2.3.5
>

Re: [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type

[Benjamin Tissoires]

Hi Jason,

On Apr 30 2015 or thereabouts, Jason Gerecke wrote:
> The naming logic currently assumes that all devices will be a pen, finger,
> or pad. Though this has historically been the case, the new HID_GENERIC
> catch-all may cause us to probe devices with Wacom's 056A VID which aren't
> any of these types (e.g. the "Cintiq 24HDT Monitor Control"). This patch
> updates the logic so that no suffix will be added to the device name if
> the device type is unknown.
> 
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> ---

I am not sure we need to split 2/3 & 3/3. But either way is fine.
I checked against a Bamboo PAD, Intuos Pro and a Bamboo Pen&Touch,
and everything seems to be fine.

The whole series is:

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

Cheers,
Benjamin

>  drivers/hid/wacom_sys.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 9c57ac0..222baf5 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -1440,12 +1440,15 @@ static void wacom_update_name(struct wacom *wacom)
>  	snprintf(wacom_wac->pad_name, sizeof(wacom_wac->pad_name),
>  		"%s Pad", wacom_wac->name);
>  
> -	if (features->device_type != BTN_TOOL_FINGER)
> +	if (features->device_type == BTN_TOOL_PEN) {
>  		strlcat(wacom_wac->name, " Pen", WACOM_NAME_MAX);
> -	else if (features->touch_max)
> -		strlcat(wacom_wac->name, " Finger", WACOM_NAME_MAX);
> -	else
> -		strlcat(wacom_wac->name, " Pad", WACOM_NAME_MAX);
> +	}
> +	else if (features->device_type == BTN_TOOL_FINGER) {
> +		if (features->touch_max)
> +			strlcat(wacom_wac->name, " Finger", WACOM_NAME_MAX);
> +		else
> +			strlcat(wacom_wac->name, " Pad", WACOM_NAME_MAX);
> +	}
>  }
>  
>  static int wacom_probe(struct hid_device *hdev,
> -- 
> 2.3.5
> 

Re: [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type

[Jiri Kosina]

On Thu, 30 Apr 2015, Jason Gerecke wrote:

> The naming logic currently assumes that all devices will be a pen, finger,
> or pad. Though this has historically been the case, the new HID_GENERIC
> catch-all may cause us to probe devices with Wacom's 056A VID which aren't
> any of these types (e.g. the "Cintiq 24HDT Monitor Control"). This patch
> updates the logic so that no suffix will be added to the device name if
> the device type is unknown.
> 
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>

I have applied the series for for-4.2/wacom.

-- 
Jiri Kosina
SUSE Labs