From: Jonathan Nieder <jrnieder@gmail.com>
To: Ping Cheng <pinglinux@gmail.com>
Cc: dmitry.torokhov@gmail.com, "Bjørn Mork" <bjorn@mork.no>,
linux-input@vger.kernel.org,
"Jason Gerecke" <killertofu@gmail.com>,
"Chris Bagwell" <chris@cnpbagwell.com>,
"Ping Cheng" <pingc@wacom.com>
Subject: [RFC/PATCH] Revert "Input: wacom - add 0xE5 (MT device) support"
Date: Thu, 14 Jun 2012 16:23:35 -0500 [thread overview]
Message-ID: <20120614212335.GA4210@burratino> (raw)
In-Reply-To: <1335402856-11256-1-git-send-email-pinglinux@gmail.com>
This reverts commit 1963518b9b1b8019d33b4b08deee6f873ffa2730.
It was supposed to just add support for new MTSCREEN devices, but
instead it significantly changed the code handling TABLETPC2FG and
BAMBOO_PT. That destroys debugability. Back to the drawing board.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hi,
Ping Cheng wrote:
> Main part of patch is adding support for a new Wacom MT touch
> packet and labels these devices using MTSCREEN type.
>
> Other items of interest:
Agh!
A patch adding new hardware support should not touch existing support
for other devices at the same time. Especially not cosmetic changes.
[1] has an analysis by Bjørn of why those unrelated changes are
probably buggy, but I don't really care about that at the moment. If
it were a separate patch with a description explaining what it was
supposed to do, the normal review process would take care of those
things. Piggy-backing onto another patch is just not a good idea.
How about this patch (untested)?
Thanks,
Jonathan
[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=55;bug=677164
drivers/input/tablet/wacom.h | 4 +-
drivers/input/tablet/wacom_sys.c | 91 +++++++++++++++--------------------
drivers/input/tablet/wacom_wac.c | 99 ++------------------------------------
drivers/input/tablet/wacom_wac.h | 8 ---
4 files changed, 45 insertions(+), 157 deletions(-)
diff --git a/drivers/input/tablet/wacom.h b/drivers/input/tablet/wacom.h
index b79d45198d82..b4842d0e61dd 100644
--- a/drivers/input/tablet/wacom.h
+++ b/drivers/input/tablet/wacom.h
@@ -135,6 +135,6 @@ extern const struct usb_device_id wacom_ids[];
void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len);
void wacom_setup_device_quirks(struct wacom_features *features);
-int wacom_setup_input_capabilities(struct input_dev *input_dev,
- struct wacom_wac *wacom_wac);
+void wacom_setup_input_capabilities(struct input_dev *input_dev,
+ struct wacom_wac *wacom_wac);
#endif
diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c
index cad5602d3ce4..a0cc46e5f13c 100644
--- a/drivers/input/tablet/wacom_sys.c
+++ b/drivers/input/tablet/wacom_sys.c
@@ -320,10 +320,6 @@ static int wacom_parse_hid(struct usb_interface *intf,
/* need to reset back */
features->pktlen = WACOM_PKGLEN_TPC2FG;
}
-
- if (features->type == MTSCREEN)
- features->pktlen = WACOM_PKGLEN_MTOUCH;
-
if (features->type == BAMBOO_PT) {
/* need to reset back */
features->pktlen = WACOM_PKGLEN_BBTOUCH;
@@ -356,15 +352,18 @@ static int wacom_parse_hid(struct usb_interface *intf,
case HID_USAGE_Y:
if (usage == WCM_DESKTOP) {
if (finger) {
- int type = features->type;
-
- if (type == TABLETPC2FG || type == MTSCREEN) {
+ features->device_type = BTN_TOOL_FINGER;
+ if (features->type == TABLETPC2FG) {
+ /* need to reset back */
+ features->pktlen = WACOM_PKGLEN_TPC2FG;
features->y_max =
get_unaligned_le16(&report[i + 3]);
features->y_phy =
get_unaligned_le16(&report[i + 6]);
i += 7;
- } else if (type == BAMBOO_PT) {
+ } else if (features->type == BAMBOO_PT) {
+ /* need to reset back */
+ features->pktlen = WACOM_PKGLEN_BBTOUCH;
features->y_phy =
get_unaligned_le16(&report[i + 3]);
features->y_max =
@@ -378,6 +377,10 @@ static int wacom_parse_hid(struct usb_interface *intf,
i += 4;
}
} else if (pen) {
+ /* penabled only accepts exact bytes of data */
+ if (features->type == TABLETPC2FG)
+ features->pktlen = WACOM_PKGLEN_GRAPHIRE;
+ features->device_type = BTN_TOOL_PEN;
features->y_max =
get_unaligned_le16(&report[i + 3]);
i += 4;
@@ -440,29 +443,22 @@ static int wacom_query_tablet_data(struct usb_interface *intf, struct wacom_feat
if (!rep_data)
return error;
- /* ask to report Wacom data */
- if (features->device_type == BTN_TOOL_FINGER) {
- /* if it is an MT Tablet PC touch */
- if (features->type == TABLETPC2FG ||
- features->type == MTSCREEN) {
- do {
- rep_data[0] = 3;
- rep_data[1] = 4;
- rep_data[2] = 0;
- rep_data[3] = 0;
- report_id = 3;
- error = wacom_set_report(intf,
- WAC_HID_FEATURE_REPORT,
- report_id,
- rep_data, 4, 1);
- if (error >= 0)
- error = wacom_get_report(intf,
- WAC_HID_FEATURE_REPORT,
- report_id,
- rep_data, 4, 1);
- } while ((error < 0 || rep_data[1] != 4) &&
- limit++ < WAC_MSG_RETRIES);
- }
+ /* ask to report tablet data if it is MT Tablet PC or
+ * not a Tablet PC */
+ if (features->type == TABLETPC2FG) {
+ do {
+ rep_data[0] = 3;
+ rep_data[1] = 4;
+ rep_data[2] = 0;
+ rep_data[3] = 0;
+ report_id = 3;
+ error = wacom_set_report(intf, WAC_HID_FEATURE_REPORT,
+ report_id, rep_data, 4, 1);
+ if (error >= 0)
+ error = wacom_get_report(intf,
+ WAC_HID_FEATURE_REPORT,
+ report_id, rep_data, 4, 1);
+ } while ((error < 0 || rep_data[1] != 4) && limit++ < WAC_MSG_RETRIES);
} else if (features->type != TABLETPC &&
features->type != WIRELESS &&
features->device_type == BTN_TOOL_PEN) {
@@ -484,7 +480,7 @@ static int wacom_query_tablet_data(struct usb_interface *intf, struct wacom_feat
}
static int wacom_retrieve_hid_descriptor(struct usb_interface *intf,
- struct wacom_features *features)
+ struct wacom_features *features)
{
int error = 0;
struct usb_host_interface *interface = intf->cur_altsetting;
@@ -512,13 +508,10 @@ static int wacom_retrieve_hid_descriptor(struct usb_interface *intf,
}
}
- /* only devices that support touch need to retrieve the info */
- if (features->type != TABLETPC &&
- features->type != TABLETPC2FG &&
- features->type != BAMBOO_PT &&
- features->type != MTSCREEN) {
+ /* only Tablet PCs and Bamboo P&T need to retrieve the info */
+ if ((features->type != TABLETPC) && (features->type != TABLETPC2FG) &&
+ (features->type != BAMBOO_PT))
goto out;
- }
error = usb_get_extra_descriptor(interface, HID_DEVICET_HID, &hid_desc);
if (error) {
@@ -990,10 +983,8 @@ static int wacom_register_input(struct wacom *wacom)
int error;
input_dev = input_allocate_device();
- if (!input_dev) {
- error = -ENOMEM;
- goto fail1;
- }
+ if (!input_dev)
+ return -ENOMEM;
input_dev->name = wacom_wac->name;
input_dev->dev.parent = &intf->dev;
@@ -1003,20 +994,14 @@ static int wacom_register_input(struct wacom *wacom)
input_set_drvdata(input_dev, wacom);
wacom_wac->input = input_dev;
- error = wacom_setup_input_capabilities(input_dev, wacom_wac);
- if (error)
- goto fail1;
+ wacom_setup_input_capabilities(input_dev, wacom_wac);
error = input_register_device(input_dev);
- if (error)
- goto fail2;
+ if (error) {
+ input_free_device(input_dev);
+ wacom_wac->input = NULL;
+ }
- return 0;
-
-fail2:
- input_free_device(input_dev);
- wacom_wac->input = NULL;
-fail1:
return error;
}
diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 004bc1bb1544..5b31418b416b 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -776,72 +776,6 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
return 1;
}
-static int find_slot_from_contactid(struct wacom_wac *wacom, int contactid)
-{
- int touch_max = wacom->features.touch_max;
- int i;
-
- if (!wacom->slots)
- return -1;
-
- for (i = 0; i < touch_max; ++i) {
- if (wacom->slots[i] == contactid)
- return i;
- }
- for (i = 0; i < touch_max; ++i) {
- if (wacom->slots[i] == -1)
- return i;
- }
- return -1;
-}
-
-static int wacom_mt_touch(struct wacom_wac *wacom)
-{
- struct input_dev *input = wacom->input;
- char *data = wacom->data;
- int i;
- int current_num_contacts = data[2];
- int contacts_to_send = 0;
-
- /*
- * First packet resets the counter since only the first
- * packet in series will have non-zero current_num_contacts.
- */
- if (current_num_contacts)
- wacom->num_contacts_left = current_num_contacts;
-
- /* There are at most 5 contacts per packet */
- contacts_to_send = min(5, wacom->num_contacts_left);
-
- for (i = 0; i < contacts_to_send; i++) {
- int offset = (WACOM_BYTES_PER_MT_PACKET * i) + 3;
- bool touch = data[offset] & 0x1;
- int id = le16_to_cpup((__le16 *)&data[offset + 1]);
- int slot = find_slot_from_contactid(wacom, id);
-
- if (slot < 0)
- continue;
-
- input_mt_slot(input, slot);
- input_mt_report_slot_state(input, MT_TOOL_FINGER, touch);
- if (touch) {
- int x = le16_to_cpup((__le16 *)&data[offset + 7]);
- int y = le16_to_cpup((__le16 *)&data[offset + 9]);
- input_report_abs(input, ABS_MT_POSITION_X, x);
- input_report_abs(input, ABS_MT_POSITION_Y, y);
- }
- wacom->slots[slot] = touch ? id : -1;
- }
-
- input_mt_report_pointer_emulation(input, true);
-
- wacom->num_contacts_left -= contacts_to_send;
- if (wacom->num_contacts_left < 0)
- wacom->num_contacts_left = 0;
-
- return 1;
-}
-
static int wacom_tpc_mt_touch(struct wacom_wac *wacom)
{
struct input_dev *input = wacom->input;
@@ -880,9 +814,6 @@ static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
bool prox;
int x = 0, y = 0;
- if (wacom->features.touch_max > 1 || len > WACOM_PKGLEN_TPC2FG)
- return 0;
-
if (!wacom->shared->stylus_in_proximity) {
if (len == WACOM_PKGLEN_TPC1FG) {
prox = data[0] & 0x01;
@@ -951,10 +882,10 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
switch (len) {
case WACOM_PKGLEN_TPC1FG:
- return wacom_tpc_single_touch(wacom, len);
+ return wacom_tpc_single_touch(wacom, len);
case WACOM_PKGLEN_TPC2FG:
- return wacom_tpc_mt_touch(wacom);
+ return wacom_tpc_mt_touch(wacom);
default:
switch (data[0]) {
@@ -963,9 +894,6 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
case WACOM_REPORT_TPCST:
return wacom_tpc_single_touch(wacom, len);
- case WACOM_REPORT_TPCMT:
- return wacom_mt_touch(wacom);
-
case WACOM_REPORT_PENABLED:
return wacom_tpc_pen(wacom);
}
@@ -1245,7 +1173,6 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
case TABLETPC:
case TABLETPC2FG:
- case MTSCREEN:
sync = wacom_tpc_irq(wacom_wac, len);
break;
@@ -1319,8 +1246,7 @@ void wacom_setup_device_quirks(struct wacom_features *features)
/* these device have multiple inputs */
if (features->type == TABLETPC || features->type == TABLETPC2FG ||
features->type == BAMBOO_PT || features->type == WIRELESS ||
- (features->type >= INTUOS5S && features->type <= INTUOS5L) ||
- features->type == MTSCREEN)
+ (features->type >= INTUOS5S && features->type <= INTUOS5L))
features->quirks |= WACOM_QUIRK_MULTI_INPUT;
/* quirk for bamboo touch with 2 low res touches */
@@ -1351,8 +1277,8 @@ static unsigned int wacom_calculate_touch_res(unsigned int logical_max,
return (logical_max * 100) / physical_max;
}
-int wacom_setup_input_capabilities(struct input_dev *input_dev,
- struct wacom_wac *wacom_wac)
+void wacom_setup_input_capabilities(struct input_dev *input_dev,
+ struct wacom_wac *wacom_wac)
{
struct wacom_features *features = &wacom_wac->features;
int i;
@@ -1548,18 +1474,8 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
break;
case TABLETPC2FG:
- case MTSCREEN:
if (features->device_type == BTN_TOOL_FINGER) {
- wacom_wac->slots = kmalloc(features->touch_max *
- sizeof(int),
- GFP_KERNEL);
- if (!wacom_wac->slots)
- return -ENOMEM;
-
- for (i = 0; i < features->touch_max; i++)
- wacom_wac->slots[i] = -1;
-
input_mt_init_slots(input_dev, features->touch_max);
input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE,
0, MT_TOOL_MAX, 0, 0);
@@ -1645,7 +1561,6 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
}
break;
}
- return 0;
}
static const struct wacom_features wacom_features_0x00 =
@@ -1878,9 +1793,6 @@ static const struct wacom_features wacom_features_0xE3 =
{ "Wacom ISDv4 E3", WACOM_PKGLEN_TPC2FG, 26202, 16325, 255,
0, TABLETPC2FG, WACOM_INTUOS_RES, WACOM_INTUOS_RES,
.touch_max = 2 };
-static const struct wacom_features wacom_features_0xE5 =
- { "Wacom ISDv4 E5", WACOM_PKGLEN_MTOUCH, 26202, 16325, 255,
- 0, MTSCREEN, WACOM_INTUOS_RES, WACOM_INTUOS_RES };
static const struct wacom_features wacom_features_0xE6 =
{ "Wacom ISDv4 E6", WACOM_PKGLEN_TPC2FG, 27760, 15694, 255,
0, TABLETPC2FG, WACOM_INTUOS_RES, WACOM_INTUOS_RES,
@@ -2059,7 +1971,6 @@ const struct usb_device_id wacom_ids[] = {
{ USB_DEVICE_WACOM(0x9F) },
{ USB_DEVICE_WACOM(0xE2) },
{ USB_DEVICE_WACOM(0xE3) },
- { USB_DEVICE_WACOM(0xE5) },
{ USB_DEVICE_WACOM(0xE6) },
{ USB_DEVICE_WACOM(0xEC) },
{ USB_DEVICE_WACOM(0x47) },
diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
index 78fbd3f42009..321269c1ac4c 100644
--- a/drivers/input/tablet/wacom_wac.h
+++ b/drivers/input/tablet/wacom_wac.h
@@ -25,10 +25,6 @@
#define WACOM_PKGLEN_BBTOUCH3 64
#define WACOM_PKGLEN_BBPEN 10
#define WACOM_PKGLEN_WIRELESS 32
-#define WACOM_PKGLEN_MTOUCH 62
-
-/* wacom data size per MT contact */
-#define WACOM_BYTES_PER_MT_PACKET 11
/* device IDs */
#define STYLUS_DEVICE_ID 0x02
@@ -45,7 +41,6 @@
#define WACOM_REPORT_INTUOS5PAD 3
#define WACOM_REPORT_TPC1FG 6
#define WACOM_REPORT_TPC2FG 13
-#define WACOM_REPORT_TPCMT 13
#define WACOM_REPORT_TPCHID 15
#define WACOM_REPORT_TPCST 16
@@ -81,7 +76,6 @@ enum {
WACOM_MO,
TABLETPC,
TABLETPC2FG,
- MTSCREEN,
MAX_TYPE
};
@@ -124,8 +118,6 @@ struct wacom_wac {
struct input_dev *input;
int pid;
int battery_capacity;
- int num_contacts_left;
- int *slots;
};
#endif
--
1.7.10.4
--
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
next prev parent reply other threads:[~2012-06-14 21:23 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-04-26 1:14 [PATCH 2/2] input: wacom - add 0xE5 (MT device) support Ping Cheng
2012-04-30 4:10 ` Dmitry Torokhov
2012-06-14 21:23 ` Jonathan Nieder [this message]
2012-06-14 23:51 ` [RFC/PATCH] Revert "Input: wacom - add 0xE5 (MT device) support" Ping Cheng
2012-06-15 0:27 ` Jonathan Nieder
2012-06-15 19:08 ` Ping Cheng
2012-06-16 3:39 ` Jonathan Nieder
2012-06-16 4:01 ` Jonathan Nieder
2012-06-19 0:06 ` Ping Cheng
2012-06-16 22:25 ` Jonathan Nieder
2012-06-17 0:43 ` Chris Bagwell
2012-06-17 21:08 ` Nils Kanning
2012-06-18 2:39 ` Chris Bagwell
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=20120614212335.GA4210@burratino \
--to=jrnieder@gmail.com \
--cc=bjorn@mork.no \
--cc=chris@cnpbagwell.com \
--cc=dmitry.torokhov@gmail.com \
--cc=killertofu@gmail.com \
--cc=linux-input@vger.kernel.org \
--cc=pingc@wacom.com \
--cc=pinglinux@gmail.com \
/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 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.