| Message ID | 1391107728-1306-1-git-send-email-killertofu@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
On Thu, Jan 30, 2014 at 10:48 AM, Jason Gerecke <killertofu@gmail.com> wrote: > > A HID Usage is a 32-bit value: an upper 16-bit "page" and a lower > 16-bit ID. While the two halves are normally reported seperately, > only the combination uniquely idenfifes a particular HID Usage. > > The existing code performs the comparison in two steps, first > performing a switch on the ID and then verifying the page within > each case. While this works fine, it is very akward to handle > two Usages that share a single ID, such as HID_USAGE_PRESSURE > and HID_USAGE_X because the case statement can only have a > single identifier. > > To work around this, we now check the full 32-bit HID Usage > directly rather than first checking the ID and then the page. > This allows the switch statement to have distinct cases for > e.g. HID_USAGE_PRESSURE and HID_USAGE_X. > > Signed-off-by: Jason Gerecke <killertofu@gmail.com> > --- > drivers/input/tablet/wacom_sys.c | 237 ++++++++++++++++++--------------------- > 1 file changed, 109 insertions(+), 128 deletions(-) > > diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c > index b16ebef..5be6177 100644 > --- a/drivers/input/tablet/wacom_sys.c > +++ b/drivers/input/tablet/wacom_sys.c > @@ -22,23 +22,17 @@ > #define HID_USAGE_PAGE_DIGITIZER 0x0d > #define HID_USAGE_PAGE_DESKTOP 0x01 > #define HID_USAGE 0x09 > -#define HID_USAGE_X 0x30 > -#define HID_USAGE_Y 0x31 > -#define HID_USAGE_X_TILT 0x3d > -#define HID_USAGE_Y_TILT 0x3e > -#define HID_USAGE_FINGER 0x22 > -#define HID_USAGE_STYLUS 0x20 > -#define HID_USAGE_CONTACTMAX 0x55 > +#define HID_USAGE_X ((HID_USAGE_PAGE_DESKTOP << 16) | 0x30) > +#define HID_USAGE_Y ((HID_USAGE_PAGE_DESKTOP << 16) | 0x31) > +#define HID_USAGE_X_TILT ((HID_USAGE_PAGE_DIGITIZER << 16) | 0x3d) > +#define HID_USAGE_Y_TILT ((HID_USAGE_PAGE_DIGITIZER << 16) | 0x3e) > +#define HID_USAGE_FINGER ((HID_USAGE_PAGE_DIGITIZER << 16) | 0x22) > +#define HID_USAGE_STYLUS ((HID_USAGE_PAGE_DIGITIZER << 16) | 0x20) > +#define HID_USAGE_CONTACTMAX ((HID_USAGE_PAGE_DIGITIZER << 16) | 0x55) > #define HID_COLLECTION 0xa1 > #define HID_COLLECTION_LOGICAL 0x02 > #define HID_COLLECTION_END 0xc0 > > -enum { > - WCM_UNDEFINED = 0, > - WCM_DESKTOP, > - WCM_DIGITIZER, > -}; > - > struct hid_descriptor { > struct usb_descriptor_header header; > __le16 bcdHID; > @@ -305,7 +299,7 @@ static int wacom_parse_hid(struct usb_interface *intf, > char limit = 0; > /* result has to be defined as int for some devices */ > int result = 0, touch_max = 0; > - int i = 0, usage = WCM_UNDEFINED, finger = 0, pen = 0; > + int i = 0, page = 0, finger = 0, pen = 0; > unsigned char *report; > > report = kzalloc(hid_desc->wDescriptorLength, GFP_KERNEL); > @@ -332,134 +326,121 @@ static int wacom_parse_hid(struct usb_interface *intf, > > switch (report[i]) { > case HID_USAGE_PAGE: > - switch (report[i + 1]) { > - case HID_USAGE_PAGE_DIGITIZER: > - usage = WCM_DIGITIZER; > - i++; > - break; > - > - case HID_USAGE_PAGE_DESKTOP: > - usage = WCM_DESKTOP; > - i++; > - break; > - } > + page = report[i + 1]; > + i++; > break; > > case HID_USAGE: > - switch (report[i + 1]) { > + switch (page << 16 | report[i + 1]) { > case HID_USAGE_X: > - if (usage == WCM_DESKTOP) { > - if (finger) { > - features->device_type = BTN_TOOL_FINGER; > - /* touch device at least supports one touch point */ > - touch_max = 1; > - switch (features->type) { > - case TABLETPC2FG: > - features->pktlen = WACOM_PKGLEN_TPC2FG; > - break; > - > - case MTSCREEN: > - case WACOM_24HDT: > - features->pktlen = WACOM_PKGLEN_MTOUCH; > - break; > - > - case MTTPC: > - features->pktlen = WACOM_PKGLEN_MTTPC; > - break; > - > - case BAMBOO_PT: > - features->pktlen = WACOM_PKGLEN_BBTOUCH; > - break; > - > - default: > - features->pktlen = WACOM_PKGLEN_GRAPHIRE; > - break; > - } > - > - switch (features->type) { > - case BAMBOO_PT: > - features->x_phy = > - get_unaligned_le16(&report[i + 5]); > - features->x_max = > - get_unaligned_le16(&report[i + 8]); > - i += 15; > - break; > - > - case WACOM_24HDT: > - features->x_max = > - get_unaligned_le16(&report[i + 3]); > - features->x_phy = > - get_unaligned_le16(&report[i + 8]); > - features->unit = report[i - 1]; > - features->unitExpo = report[i - 3]; > - i += 12; > - break; > - > - default: > - features->x_max = > - get_unaligned_le16(&report[i + 3]); > - features->x_phy = > - get_unaligned_le16(&report[i + 6]); > - features->unit = report[i + 9]; > - features->unitExpo = report[i + 11]; > - i += 12; > - break; > - } > - } else if (pen) { > - /* penabled only accepts exact bytes of data */ > - if (features->type >= TABLETPC) > - features->pktlen = WACOM_PKGLEN_GRAPHIRE; > - features->device_type = BTN_TOOL_PEN; > + if (finger) { > + features->device_type = BTN_TOOL_FINGER; > + /* touch device at least supports one touch point */ > + touch_max = 1; > + switch (features->type) { > + case TABLETPC2FG: > + features->pktlen = WACOM_PKGLEN_TPC2FG; > + break; > + > + case MTSCREEN: > + case WACOM_24HDT: > + features->pktlen = WACOM_PKGLEN_MTOUCH; > + break; > + > + case MTTPC: > + features->pktlen = WACOM_PKGLEN_MTTPC; > + break; > + > + case BAMBOO_PT: > + features->pktlen = WACOM_PKGLEN_BBTOUCH; > + break; > + > + default: > + features->pktlen = WACOM_PKGLEN_GRAPHIRE; > + break; > + } > + > + switch (features->type) { > + case BAMBOO_PT: > + features->x_phy = > + get_unaligned_le16(&report[i + 5]); > + features->x_max = > + get_unaligned_le16(&report[i + 8]); > + i += 15; > + break; > + > + case WACOM_24HDT: > features->x_max = > get_unaligned_le16(&report[i + 3]); > - i += 4; > + features->x_phy = > + get_unaligned_le16(&report[i + 8]); > + features->unit = report[i - 1]; > + features->unitExpo = report[i - 3]; > + i += 12; > + break; > + > + default: > + features->x_max = > + get_unaligned_le16(&report[i + 3]); > + features->x_phy = > + get_unaligned_le16(&report[i + 6]); > + features->unit = report[i + 9]; > + features->unitExpo = report[i + 11]; > + i += 12; > + break; > } > + } else if (pen) { > + /* penabled only accepts exact bytes of data */ > + if (features->type >= TABLETPC) > + features->pktlen = WACOM_PKGLEN_GRAPHIRE; > + features->device_type = BTN_TOOL_PEN; > + features->x_max = > + get_unaligned_le16(&report[i + 3]); > + i += 4; > } > break; > > case HID_USAGE_Y: > - if (usage == WCM_DESKTOP) { > - if (finger) { > - switch (features->type) { > - case TABLETPC2FG: > - case MTSCREEN: > - case MTTPC: > - features->y_max = > - get_unaligned_le16(&report[i + 3]); > - features->y_phy = > - get_unaligned_le16(&report[i + 6]); > - i += 7; > - break; > - > - case WACOM_24HDT: > - features->y_max = > - get_unaligned_le16(&report[i + 3]); > - features->y_phy = > - get_unaligned_le16(&report[i - 2]); > - i += 7; > - break; > - > - case BAMBOO_PT: > - features->y_phy = > - get_unaligned_le16(&report[i + 3]); > - features->y_max = > - get_unaligned_le16(&report[i + 6]); > - i += 12; > - break; > - > - default: > - features->y_max = > - features->x_max; > - features->y_phy = > - get_unaligned_le16(&report[i + 3]); > - i += 4; > - break; > - } > - } else if (pen) { > + if (finger) { > + switch (features->type) { > + case TABLETPC2FG: > + case MTSCREEN: > + case MTTPC: > + features->y_max = > + get_unaligned_le16(&report[i + 3]); > + features->y_phy = > + get_unaligned_le16(&report[i + 6]); > + i += 7; > + break; > + > + case WACOM_24HDT: > + features->y_max = > + get_unaligned_le16(&report[i + 3]); > + features->y_phy = > + get_unaligned_le16(&report[i - 2]); > + i += 7; > + break; > + > + case BAMBOO_PT: > + features->y_phy = > + get_unaligned_le16(&report[i + 3]); > + features->y_max = > + get_unaligned_le16(&report[i + 6]); > + i += 12; > + break; > + > + default: > features->y_max = > + features->x_max; > + features->y_phy = > get_unaligned_le16(&report[i + 3]); > i += 4; > + break; > } > + } else if (pen) { > + features->y_max = > + get_unaligned_le16(&report[i + 3]); > + i += 4; > } > break; > > @@ -489,7 +470,7 @@ static int wacom_parse_hid(struct usb_interface *intf, > > case HID_COLLECTION_END: > /* reset UsagePage and Finger */ > - finger = usage = 0; > + finger = page = 0; > break; > > case HID_COLLECTION: > -- > 1.8.5.3 > Tested-by: Aaron Skomra <Aaron.Skomra@wacom.com> -- 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
diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c index b16ebef..5be6177 100644 --- a/drivers/input/tablet/wacom_sys.c +++ b/drivers/input/tablet/wacom_sys.c @@ -22,23 +22,17 @@ #define HID_USAGE_PAGE_DIGITIZER 0x0d #define HID_USAGE_PAGE_DESKTOP 0x01 #define HID_USAGE 0x09 -#define HID_USAGE_X 0x30 -#define HID_USAGE_Y 0x31 -#define HID_USAGE_X_TILT 0x3d -#define HID_USAGE_Y_TILT 0x3e -#define HID_USAGE_FINGER 0x22 -#define HID_USAGE_STYLUS 0x20 -#define HID_USAGE_CONTACTMAX 0x55 +#define HID_USAGE_X ((HID_USAGE_PAGE_DESKTOP << 16) | 0x30) +#define HID_USAGE_Y ((HID_USAGE_PAGE_DESKTOP << 16) | 0x31) +#define HID_USAGE_X_TILT ((HID_USAGE_PAGE_DIGITIZER << 16) | 0x3d) +#define HID_USAGE_Y_TILT ((HID_USAGE_PAGE_DIGITIZER << 16) | 0x3e) +#define HID_USAGE_FINGER ((HID_USAGE_PAGE_DIGITIZER << 16) | 0x22) +#define HID_USAGE_STYLUS ((HID_USAGE_PAGE_DIGITIZER << 16) | 0x20) +#define HID_USAGE_CONTACTMAX ((HID_USAGE_PAGE_DIGITIZER << 16) | 0x55) #define HID_COLLECTION 0xa1 #define HID_COLLECTION_LOGICAL 0x02 #define HID_COLLECTION_END 0xc0 -enum { - WCM_UNDEFINED = 0, - WCM_DESKTOP, - WCM_DIGITIZER, -}; - struct hid_descriptor { struct usb_descriptor_header header; __le16 bcdHID; @@ -305,7 +299,7 @@ static int wacom_parse_hid(struct usb_interface *intf, char limit = 0; /* result has to be defined as int for some devices */ int result = 0, touch_max = 0; - int i = 0, usage = WCM_UNDEFINED, finger = 0, pen = 0; + int i = 0, page = 0, finger = 0, pen = 0; unsigned char *report; report = kzalloc(hid_desc->wDescriptorLength, GFP_KERNEL); @@ -332,134 +326,121 @@ static int wacom_parse_hid(struct usb_interface *intf, switch (report[i]) { case HID_USAGE_PAGE: - switch (report[i + 1]) { - case HID_USAGE_PAGE_DIGITIZER: - usage = WCM_DIGITIZER; - i++; - break; - - case HID_USAGE_PAGE_DESKTOP: - usage = WCM_DESKTOP; - i++; - break; - } + page = report[i + 1]; + i++; break; case HID_USAGE: - switch (report[i + 1]) { + switch (page << 16 | report[i + 1]) { case HID_USAGE_X: - if (usage == WCM_DESKTOP) { - if (finger) { - features->device_type = BTN_TOOL_FINGER; - /* touch device at least supports one touch point */ - touch_max = 1; - switch (features->type) { - case TABLETPC2FG: - features->pktlen = WACOM_PKGLEN_TPC2FG; - break; - - case MTSCREEN: - case WACOM_24HDT: - features->pktlen = WACOM_PKGLEN_MTOUCH; - break; - - case MTTPC: - features->pktlen = WACOM_PKGLEN_MTTPC; - break; - - case BAMBOO_PT: - features->pktlen = WACOM_PKGLEN_BBTOUCH; - break; - - default: - features->pktlen = WACOM_PKGLEN_GRAPHIRE; - break; - } - - switch (features->type) { - case BAMBOO_PT: - features->x_phy = - get_unaligned_le16(&report[i + 5]); - features->x_max = - get_unaligned_le16(&report[i + 8]); - i += 15; - break; - - case WACOM_24HDT: - features->x_max = - get_unaligned_le16(&report[i + 3]); - features->x_phy = - get_unaligned_le16(&report[i + 8]); - features->unit = report[i - 1]; - features->unitExpo = report[i - 3]; - i += 12; - break; - - default: - features->x_max = - get_unaligned_le16(&report[i + 3]); - features->x_phy = - get_unaligned_le16(&report[i + 6]); - features->unit = report[i + 9]; - features->unitExpo = report[i + 11]; - i += 12; - break; - } - } else if (pen) { - /* penabled only accepts exact bytes of data */ - if (features->type >= TABLETPC) - features->pktlen = WACOM_PKGLEN_GRAPHIRE; - features->device_type = BTN_TOOL_PEN; + if (finger) { + features->device_type = BTN_TOOL_FINGER; + /* touch device at least supports one touch point */ + touch_max = 1; + switch (features->type) { + case TABLETPC2FG: + features->pktlen = WACOM_PKGLEN_TPC2FG; + break; + + case MTSCREEN: + case WACOM_24HDT: + features->pktlen = WACOM_PKGLEN_MTOUCH; + break; + + case MTTPC: + features->pktlen = WACOM_PKGLEN_MTTPC; + break; + + case BAMBOO_PT: + features->pktlen = WACOM_PKGLEN_BBTOUCH; + break; + + default: + features->pktlen = WACOM_PKGLEN_GRAPHIRE; + break; + } + + switch (features->type) { + case BAMBOO_PT: + features->x_phy = + get_unaligned_le16(&report[i + 5]); + features->x_max = + get_unaligned_le16(&report[i + 8]); + i += 15; + break; + + case WACOM_24HDT: features->x_max = get_unaligned_le16(&report[i + 3]); - i += 4; + features->x_phy = + get_unaligned_le16(&report[i + 8]); + features->unit = report[i - 1]; + features->unitExpo = report[i - 3]; + i += 12; + break; + + default: + features->x_max = + get_unaligned_le16(&report[i + 3]); + features->x_phy = + get_unaligned_le16(&report[i + 6]); + features->unit = report[i + 9]; + features->unitExpo = report[i + 11]; + i += 12; + break; } + } else if (pen) { + /* penabled only accepts exact bytes of data */ + if (features->type >= TABLETPC) + features->pktlen = WACOM_PKGLEN_GRAPHIRE; + features->device_type = BTN_TOOL_PEN; + features->x_max = + get_unaligned_le16(&report[i + 3]); + i += 4; } break; case HID_USAGE_Y: - if (usage == WCM_DESKTOP) { - if (finger) { - switch (features->type) { - case TABLETPC2FG: - case MTSCREEN: - case MTTPC: - features->y_max = - get_unaligned_le16(&report[i + 3]); - features->y_phy = - get_unaligned_le16(&report[i + 6]); - i += 7; - break; - - case WACOM_24HDT: - features->y_max = - get_unaligned_le16(&report[i + 3]); - features->y_phy = - get_unaligned_le16(&report[i - 2]); - i += 7; - break; - - case BAMBOO_PT: - features->y_phy = - get_unaligned_le16(&report[i + 3]); - features->y_max = - get_unaligned_le16(&report[i + 6]); - i += 12; - break; - - default: - features->y_max = - features->x_max; - features->y_phy = - get_unaligned_le16(&report[i + 3]); - i += 4; - break; - } - } else if (pen) { + if (finger) { + switch (features->type) { + case TABLETPC2FG: + case MTSCREEN: + case MTTPC: + features->y_max = + get_unaligned_le16(&report[i + 3]); + features->y_phy = + get_unaligned_le16(&report[i + 6]); + i += 7; + break; + + case WACOM_24HDT: + features->y_max = + get_unaligned_le16(&report[i + 3]); + features->y_phy = + get_unaligned_le16(&report[i - 2]); + i += 7; + break; + + case BAMBOO_PT: + features->y_phy = + get_unaligned_le16(&report[i + 3]); + features->y_max = + get_unaligned_le16(&report[i + 6]); + i += 12; + break; + + default: features->y_max = + features->x_max; + features->y_phy = get_unaligned_le16(&report[i + 3]); i += 4; + break; } + } else if (pen) { + features->y_max = + get_unaligned_le16(&report[i + 3]); + i += 4; } break; @@ -489,7 +470,7 @@ static int wacom_parse_hid(struct usb_interface *intf, case HID_COLLECTION_END: /* reset UsagePage and Finger */ - finger = usage = 0; + finger = page = 0; break; case HID_COLLECTION:
A HID Usage is a 32-bit value: an upper 16-bit "page" and a lower 16-bit ID. While the two halves are normally reported seperately, only the combination uniquely idenfifes a particular HID Usage. The existing code performs the comparison in two steps, first performing a switch on the ID and then verifying the page within each case. While this works fine, it is very akward to handle two Usages that share a single ID, such as HID_USAGE_PRESSURE and HID_USAGE_X because the case statement can only have a single identifier. To work around this, we now check the full 32-bit HID Usage directly rather than first checking the ID and then the page. This allows the switch statement to have distinct cases for e.g. HID_USAGE_PRESSURE and HID_USAGE_X. Signed-off-by: Jason Gerecke <killertofu@gmail.com> --- drivers/input/tablet/wacom_sys.c | 237 ++++++++++++++++++--------------------- 1 file changed, 109 insertions(+), 128 deletions(-)