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 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).