HID: wacom: Do not add suffix to name of devices with an unknown type
Subsystems (2)
PATCH 1/3 HID: wacom: Do not add suffix to name of devices with an unknown type
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.
--- drivers/hid/wacom_sys.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
drivers/hid/wacom_sys.c +8 -5
@@ -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,
Signature · 1 line
2.3.5
PATCH 2/3 HID: wacom: Discover device_type from HID descriptor for all devices
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.
--- 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(-)
drivers/hid/wacom_sys.c +13 -10
@@ -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);
drivers/hid/wacom_wac.c +5 -3
@@ -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 ;
drivers/hid/wacom_wac.h +5 -1
@@ -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))
Signature · 1 line
2.3.5
PATCH 3/3 HID: wacom: Fail probe if HID_GENERIC device has unknown device_type
In reply to Jason GereckeThe 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.
--- drivers/hid/wacom_sys.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/hid/wacom_sys.c +7 -1
@@ -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 ; }
Signature · 1 line
2.3.5
Quoted 8 lines · 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_GENERICcatch-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 <[email protected]>
Cheers,
Ping
Quoted 32 lines
--- drivers/hid/wacom_sys.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)drivers/hid/wacom_sys.c +8
-6@@ -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
Hi Jason,
Quoted 9 lines · 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_GENERICcatch-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 <[email protected]>---
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:
Cheers,
Benjamin
Quoted 31 lines
drivers/hid/wacom_sys.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)drivers/hid/wacom_sys.c +8
-5@@ -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,Signature · 1 line
2.3.5
Quoted 8 lines · 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_GENERICcatch-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 <[email protected]>
I have applied the series for for-4.2/wacom.
Signature · 2 lines
Jiri Kosina SUSE Labs