From: Aaron Armstrong Skomra <skomra@gmail.com>
To: linux-input@vger.kernel.org
Subject: Re: [PATCH 1/4] Input: wacom: Use full 32-bit HID Usage value in switch statement
Date: Thu, 27 Feb 2014 10:37:39 -0800 [thread overview]
Message-ID: <CAEoswT2KtMFQgg8xpJmMT4VRQTAQDHz3hasWtPmPzuc0pFemvg@mail.gmail.com> (raw)
In-Reply-To: <1391107728-1306-1-git-send-email-killertofu@gmail.com>
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>
next prev parent reply other threads:[~2014-02-27 18:37 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-30 18:48 [PATCH 1/4] Input: wacom: Use full 32-bit HID Usage value in switch statement Jason Gerecke
2014-01-30 18:48 ` [PATCH 2/4] Input: wacom: Override 'pressure_max' with value from HID_USAGE_PRESSURE Jason Gerecke
[not found] ` <1391107728-1306-2-git-send-email-killertofu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-02-27 18:27 ` Aaron Armstrong Skomra
2014-02-27 18:39 ` Aaron Armstrong Skomra
2014-01-30 18:48 ` [PATCH 3/4] Input: wacom: References to 'wacom->data' should use 'unsigned char*' Jason Gerecke
[not found] ` <1391107728-1306-3-git-send-email-killertofu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-02-27 18:28 ` Aaron Armstrong Skomra
2014-02-27 18:40 ` Aaron Armstrong Skomra
2014-01-30 18:48 ` [PATCH 4/4] Input: wacom: Handle 1024 pressure levels in wacom_tpc_pen Jason Gerecke
[not found] ` <1391107728-1306-4-git-send-email-killertofu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-02-27 18:28 ` Aaron Armstrong Skomra
2014-02-27 18:40 ` Aaron Armstrong Skomra
2014-02-27 18:37 ` Aaron Armstrong Skomra [this message]
-- strict thread matches above, loose matches on Subject: below --
2014-03-04 21:10 [PATCH 1/4] Input: wacom: Use full 32-bit HID Usage value in switch statement Ping Cheng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAEoswT2KtMFQgg8xpJmMT4VRQTAQDHz3hasWtPmPzuc0pFemvg@mail.gmail.com \
--to=skomra@gmail.com \
--cc=linux-input@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).