All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type
@ 2015-05-01  0:51 Jason Gerecke
  2015-05-01  0:51 ` [PATCH 2/3] HID: wacom: Discover device_type from HID descriptor for all devices Jason Gerecke
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Jason Gerecke @ 2015-05-01  0:51 UTC (permalink / raw)
  To: Jiri Kosina, Ping Cheng, Benjamin Tissoires
  Cc: linux-input, linux-kernel, Jason Gerecke, 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] HID: wacom: Discover device_type from HID descriptor for all devices
  2015-05-01  0:51 [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type Jason Gerecke
@ 2015-05-01  0:51 ` Jason Gerecke
  2015-05-01  0:51 ` [PATCH 3/3] HID: wacom: Fail probe if HID_GENERIC device has unknown device_type Jason Gerecke
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jason Gerecke @ 2015-05-01  0:51 UTC (permalink / raw)
  To: Jiri Kosina, Ping Cheng, Benjamin Tissoires
  Cc: linux-input, linux-kernel, Jason Gerecke, 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] HID: wacom: Fail probe if HID_GENERIC device has unknown device_type
  2015-05-01  0:51 [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type Jason Gerecke
  2015-05-01  0:51 ` [PATCH 2/3] HID: wacom: Discover device_type from HID descriptor for all devices Jason Gerecke
@ 2015-05-01  0:51 ` Jason Gerecke
  2015-05-01  1:20 ` [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type Ping Cheng
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jason Gerecke @ 2015-05-01  0:51 UTC (permalink / raw)
  To: Jiri Kosina, Ping Cheng, Benjamin Tissoires
  Cc: linux-input, linux-kernel, Jason Gerecke, 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type
  2015-05-01  0:51 [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type Jason Gerecke
  2015-05-01  0:51 ` [PATCH 2/3] HID: wacom: Discover device_type from HID descriptor for all devices Jason Gerecke
  2015-05-01  0:51 ` [PATCH 3/3] HID: wacom: Fail probe if HID_GENERIC device has unknown device_type Jason Gerecke
@ 2015-05-01  1:20 ` Ping Cheng
  2015-05-01 20:35 ` Benjamin Tissoires
  2015-05-04  8:02 ` Jiri Kosina
  4 siblings, 0 replies; 6+ messages in thread
From: Ping Cheng @ 2015-05-01  1:20 UTC (permalink / raw)
  To: Jason Gerecke
  Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
	Jason Gerecke

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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type
  2015-05-01  0:51 [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type Jason Gerecke
                   ` (2 preceding siblings ...)
  2015-05-01  1:20 ` [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type Ping Cheng
@ 2015-05-01 20:35 ` Benjamin Tissoires
  2015-05-04  8:02 ` Jiri Kosina
  4 siblings, 0 replies; 6+ messages in thread
From: Benjamin Tissoires @ 2015-05-01 20:35 UTC (permalink / raw)
  To: Jason Gerecke
  Cc: Jiri Kosina, Ping Cheng, linux-input, linux-kernel, Jason Gerecke

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
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type
  2015-05-01  0:51 [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type Jason Gerecke
                   ` (3 preceding siblings ...)
  2015-05-01 20:35 ` Benjamin Tissoires
@ 2015-05-04  8:02 ` Jiri Kosina
  4 siblings, 0 replies; 6+ messages in thread
From: Jiri Kosina @ 2015-05-04  8:02 UTC (permalink / raw)
  To: Jason Gerecke
  Cc: Ping Cheng, Benjamin Tissoires, linux-input, linux-kernel, Jason Gerecke

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-05-04  8:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-01  0:51 [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type Jason Gerecke
2015-05-01  0:51 ` [PATCH 2/3] HID: wacom: Discover device_type from HID descriptor for all devices Jason Gerecke
2015-05-01  0:51 ` [PATCH 3/3] HID: wacom: Fail probe if HID_GENERIC device has unknown device_type Jason Gerecke
2015-05-01  1:20 ` [PATCH 1/3] HID: wacom: Do not add suffix to name of devices with an unknown type Ping Cheng
2015-05-01 20:35 ` Benjamin Tissoires
2015-05-04  8:02 ` Jiri Kosina

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.