[PATCH 1/7] HID: N-trig MTM Driver fix and cleanup patch 1

[PATCH 1/7] HID: N-trig MTM Driver fix and cleanup patch 1

[mickib1]

From: micki <micki@micki-laptop.(none)>

Add Change Log and defines of MTM firmware.
Add debug Paramater change Driver name in hid_driver structure

N-trig is changing the way people interact with computers by providing a dual-mode pen and true multi-touch input device, specifically designed for today's advanced computing world.
N-trig DuoSense® solution provides a real Hands-on computing® experience, and sets the stage for OEMs and ISVs to introduce innovative computer products and applications for an intuitive, Hands-on® experience directly onscreen.
DuoSense digitizers are easily integrated into existing technologies, support all LCDs, keep devices slim and light, and can be implemented in a broad range of products, ranging from small notebooks to large LCDs.
N-trig has offices in Israel, the US, Taiwan and Japan.

Signed-off-by: Micki Balanga <micki@n-trig.com>
---
 drivers/hid/hid-ntrig.c |   94 ++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 3234c72..e99342d 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -3,7 +3,18 @@
  *
  *  Copyright (c) 2008 Rafi Rubin
  *  Copyright (c) 2009 Stephane Chatty
+ *  Copyright (c) 2010 N-TRIG
  *
+ *	1.0 - Rafi Rubin ( Version From 2010-02-13 02:13:05)
+ *		This cleans up the identification of multitouch
+ *		groups and enables the end of group sync.
+ *		Taps are now explicitly handled to adjust for the changes in the
+ *		event stream in multitouch mode.
+ *		Added triple and quad tap for the benefit of
+ *		tools that recognize different tap types but
+ *		do not have full multi touch support.
+ *	1.1 - N-trig - Add Change Log and defines of MTM firmware.
+ *		Add debug Paramater change Driver name in hid_driver structure
  */
 
 /*
@@ -16,8 +27,78 @@
 #include <linux/device.h>
 #include <linux/hid.h>
 #include <linux/module.h>
-
+#include <linux/usb.h>
 #include "hid-ids.h"
+#include "usbhid/usbhid.h"
+
+/*
+ * Pen Event Field Declaration
+ */
+#define	EVENT_PEN_TIP		0x03
+#define	EVENT_PEN_RIGHT		0x05
+#define	EVENT_TOUCH_PEN		0x07
+#define EVENT_PEN_IN_RANGE	0x01
+
+/*
+ * MTM 4 last bytes of report descriptor
+ */
+#define REPORT_GENERIC1		0x01
+#define REPORT_MT		0x02
+#define REPORT_PALM		0x03
+#define REPORT_GENERIC2		0x04
+
+#define NTRIG_USB_DEVICE_ID	0x0001
+
+/*
+ * MTM fields
+ */
+#define PEN_REPORT_SIZE		0x48
+#define MAX_FINGERS_SUPPORT	0x06
+#define END_OF_REPORT		0x64
+
+/*
+ * Dummy Finger Declaration
+ */
+#define X_CORD_VAL		0x00
+#define Y_CORD_VAL		0x00
+#define DX_CORD_VAL		0xFA
+#define DY_CORD_VAL		0x96
+#define GENERIC_BYTE_VAL	0x0D
+
+/*
+ * MTM Parse Event
+ */
+#define MTM_FRAME_INDEX		0xff000001
+#define MTM_PROPROETARY		0xff000002
+
+/*
+ * MTM  Set Feature Commands
+ */
+#define REPORTID_DRIVER_ALIVE	0x0A
+#define REPORTID_CALIBRATION	0x0B
+#define REPORTID_GET_VERSION	0x0C
+#define REPORTID_GET_MODE	0x0D
+#define REPORTID_SET_MODE_PEN	0x0E
+#define REPORTID_SET_MODE_TOUCH	0x0F
+#define REPORTID_SET_MODE_DUAL	0x10
+#define REPORTID_CALIBRATION_RESPOND	0x11
+#define HID_CAPACITORS_CALIB	0x12
+#define HID_GET_CAPACITORS_CALIB_DONE	0x13
+
+
+static int debug;
+
+#define MODULE_NAME "hid_ntrig"
+
+
+#define info(format, arg...) \
+	printk(KERN_INFO "%s: " format , MODULE_NAME , ## arg)
+#define ntrig_dbg(format, arg...) \
+	do { \
+		if (debug) \
+			printk(KERN_DEBUG "%s: " format, \
+			MODULE_NAME , ## arg); \
+	} while (0)
 
 #define NTRIG_DUPLICATE_USAGES	0x001
 
@@ -123,8 +204,8 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
  * decide whether we are in multi or single touch mode
  * and call input_mt_sync after each point if necessary
  */
-static int ntrig_event (struct hid_device *hid, struct hid_field *field,
-		                        struct hid_usage *usage, __s32 value)
+static int ntrig_event(struct hid_device *hid, struct hid_field *field,
+			struct hid_usage *usage, __s32 value)
 {
 	struct input_dev *input = field->hidinput->input;
 	struct ntrig_data *nd = hid_get_drvdata(hid);
@@ -133,7 +214,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 	if (field->application == HID_DG_PEN)
 		return 0;
 
-        if (hid->claimed & HID_CLAIMED_INPUT) {
+	if (hid->claimed & HID_CLAIMED_INPUT) {
 		switch (usage->hid) {
 		case 0xff000001:
 			/* Tag indicating the start of a multitouch group */
@@ -362,7 +443,7 @@ static const struct hid_usage_id ntrig_grabbed_usages[] = {
 };
 
 static struct hid_driver ntrig_driver = {
-	.name = "ntrig",
+	.name = MODULE_NAME,
 	.id_table = ntrig_devices,
 	.probe = ntrig_probe,
 	.remove = ntrig_remove,
@@ -384,4 +465,7 @@ static void __exit ntrig_exit(void)
 
 module_init(ntrig_init);
 module_exit(ntrig_exit);
+
+MODULE_PARM_DESC(debug, "Debug mode enable disable");
+module_param(debug, bool, 0644);
 MODULE_LICENSE("GPL");
-- 
1.6.3.3

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

[PATCH 2/7] HID: N-trig MTM Driver fix and cleanup patch 2

[mickib1]

From: micki <micki@micki-laptop.(none)>

Change ntrig_input_mapping function

N-trig is changing the way people interact with computers by providing a dual-mode pen and true multi-touch input device, specifically designed for today's advanced computing world.
N-trig DuoSense® solution provides a real Hands-on computing® experience, and sets the stage for OEMs and ISVs to introduce innovative computer products and applications for an intuitive, Hands-on® experience directly onscreen.
DuoSense digitizers are easily integrated into existing technologies, support all LCDs, keep devices slim and light, and can be implemented in a broad range of products, ranging from small notebooks to large LCDs.
N-trig has offices in Israel, the US, Taiwan and Japan.

Signed-off-by: Micki Balanga <micki@n-trig.com>
---
 drivers/hid/hid-ntrig.c |   23 +++++++++--------------
 1 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index e99342d..3602cd0 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -15,6 +15,7 @@
  *		do not have full multi touch support.
  *	1.1 - N-trig - Add Change Log and defines of MTM firmware.
  *		Add debug Paramater change Driver name in hid_driver structure
+ *	1.2 - N-trig - Change ntrig_input_mapping
  */
 
 /*
@@ -118,21 +119,12 @@ struct ntrig_data {
 	__u8 mt_foot_count;
 };
 
-/*
- * this driver is aimed at two firmware versions in circulation:
- *  - dual pen/finger single touch
- *  - finger multitouch, pen not working
- */
-
 static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
-	/* No special mappings needed for the pen and single touch */
-	if (field->physical)
-		return 0;
-
 	switch (usage->hid & HID_USAGE_PAGE) {
+
 	case HID_UP_GENDESK:
 		switch (usage->hid) {
 		case HID_GD_X:
@@ -155,12 +147,15 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	case HID_UP_DIGITIZER:
 		switch (usage->hid) {
 		/* we do not want to map these for now */
-		case HID_DG_CONTACTID: /* Not trustworthy, squelch for now */
+		case HID_DG_INVERT: /* value is always 0 */
+		case HID_DG_ERASER: /* value is always 0 */
+		case HID_DG_CONTACTID: /* value is useless */
+		case HID_DG_BARRELSWITCH:  /* doubtful */
 		case HID_DG_INPUTMODE:
 		case HID_DG_DEVICEINDEX:
+		case HID_DG_CONTACTCOUNT:
 		case HID_DG_CONTACTMAX:
 			return -1;
-
 		/* width/height mapped on TouchMajor/TouchMinor/Orientation */
 		case HID_DG_WIDTH:
 			hid_map_usage(hi, usage, bit, max,
@@ -169,8 +164,8 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		case HID_DG_HEIGHT:
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_TOUCH_MINOR);
-			input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
-					0, 1, 0, 0);
+			hid_map_usage(hi, usage, bit, max,
+					EV_ABS, ABS_MT_TRACKING_ID);
 			return 1;
 		}
 		return 0;
-- 
1.6.3.3

[PATCH 3/7] HID: N-trig MTM Driver fix And cleanup patch 3

[mickib1]

From: micki <micki@micki-laptop.(none)>

Change ntrig_input_mapped function to support EV_MSC events.
Support pen button left/right

N-trig is changing the way people interact with computers by providing a dual-mode pen and true multi-touch input device, specifically designed for today's advanced computing world.
N-trig DuoSense® solution provides a real Hands-on computing® experience, and sets the stage for OEMs and ISVs to introduce innovative computer products and applications for an intuitive, Hands-on® experience directly onscreen.
DuoSense digitizers are easily integrated into existing technologies, support all LCDs, keep devices slim and light, and can be implemented in a broad range of products, ranging from small notebooks to large LCDs.
N-trig has offices in Israel, the US, Taiwan and Japan.

Signed-off-by: Micki Balanga <micki@n-trig.com>
---
 drivers/hid/hid-ntrig.c |   26 ++++++++++++++++++--------
 1 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 3602cd0..8130c3c 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -16,6 +16,7 @@
  *	1.1 - N-trig - Add Change Log and defines of MTM firmware.
  *		Add debug Paramater change Driver name in hid_driver structure
  *	1.2 - N-trig - Change ntrig_input_mapping
+ *	1.3 - N-trig - Change ntrig_input_mapped
  */
 
 /*
@@ -182,14 +183,23 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
-	/* No special mappings needed for the pen and single touch */
-	if (field->physical)
-		return 0;
-
-	if (usage->type == EV_KEY || usage->type == EV_REL
-			|| usage->type == EV_ABS)
-		clear_bit(usage->code, *bit);
-
+       /*
+	* Maps Key For Pen And Touch events
+	* MSC events used to transfer information about finger status
+	* In cureent Frame
+	* todo: Maybe later transfer to mt proctol
+	* if there will be relevant events
+	*/
+	struct input_dev *input = hi->input;
+	set_bit(BTN_LEFT, input->keybit);
+	set_bit(BTN_RIGHT, input->keybit);
+	set_bit(EV_MSC, input->evbit);
+	set_bit(MSC_PULSELED, input->mscbit);
+	set_bit(MSC_SERIAL, input->mscbit);
+	set_bit(MSC_SCAN, input->mscbit);
+
+	input_set_abs_params(hi->input, ABS_PRESSURE,
+			0/*Min*/, 255 /*Max*/, 0, 0);
 	return 0;
 }
 
-- 
1.6.3.3

[PATCH 4/7] HID: N-trig MTM Driver fix And cleanup Patch 4

[mickib1]

From: micki <micki@micki-laptop.(none)>

Add ntrig_send_report - enable us to configure firmware.

N-trig is changing the way people interact with computers by providing a dual-mode pen and true multi-touch input device, specifically designed for today's advanced computing world.
N-trig DuoSense® solution provides a real Hands-on computing® experience, and sets the stage for OEMs and ISVs to introduce innovative computer products and applications for an intuitive, Hands-on® experience directly onscreen.
DuoSense digitizers are easily integrated into existing technologies, support all LCDs, keep devices slim and light, and can be implemented in a broad range of products, ranging from small notebooks to large LCDs.
N-trig has offices in Israel, the US, Taiwan and Japan.

Signed-off-by: Micki Balanga <micki@n-trig.com>
---
 drivers/hid/hid-ntrig.c |   50 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 8130c3c..8cdb3c2 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -17,6 +17,8 @@
  *		Add debug Paramater change Driver name in hid_driver structure
  *	1.2 - N-trig - Change ntrig_input_mapping
  *	1.3 - N-trig - Change ntrig_input_mapped
+ *	1.4 - N-trig - Add ntrig_send_report function
+ *		send set feature command to firmware
  */
 
 /*
@@ -361,6 +363,54 @@ static int ntrig_event(struct hid_device *hid, struct hid_field *field,
 	return 1;
 }
 
+/*
+ * This function used to configure N-trig firmware
+ * The first command we need to send to firmware is change
+ * to Multi-touch Mode we don't receive a reply
+ */
+static int ntrig_send_report(struct hid_device *hid)
+{
+	struct hid_report *report;
+	struct list_head *report_list =
+			&hid->report_enum[HID_FEATURE_REPORT].report_list;
+
+	if (list_empty(report_list)) {
+		ntrig_dbg("no feature reports found\n");
+		return -ENODEV;
+	}
+	report = list_first_entry(report_list, struct hid_report, list);
+	if (report->maxfield < 1)
+		return -ENODEV;
+
+	list_for_each_entry(report,
+			    report_list, list) {
+		if (report->maxfield < 1) {
+		      ntrig_dbg("no fields in the report\n");
+		      continue;
+		}
+		ntrig_dbg("Report ID %x\n", report->id);
+		switch (report->id) {
+		case REPORTID_DRIVER_ALIVE:
+		      usbhid_submit_report(hid, report, USB_DIR_OUT);
+		      break;
+		      /*
+		       * These command will implement later accroding to demand
+		       */
+		case REPORTID_GET_VERSION:	/* FALLTHRU */
+		case REPORTID_SET_MODE_DUAL:	/* FALLTHRU */
+		case REPORTID_CALIBRATION:	/* FALLTHRU */
+		case REPORTID_GET_MODE:		/* FALLTHRU */
+		case REPORTID_SET_MODE_PEN:	/* FALLTHRU */
+		case REPORTID_SET_MODE_TOUCH:	/* FALLTHRU */
+		case REPORTID_CALIBRATION_RESPOND:/* FALLTHRU */
+		case HID_CAPACITORS_CALIB:	/* FALLTHRU */
+		case HID_GET_CAPACITORS_CALIB_DONE:
+		      break;
+		}
+	}
+	return 0;
+}
+
 static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	int ret;
-- 
1.6.3.3

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

[PATCH 5/7] HID: N-trig MTM Driver fix And cleanup patch 5

[mickib1]

From: micki <micki@micki-laptop.(none)>

udpate ntrig_probe - add call to ntrig_send_report function
to initialize firmware to switch to Multi-touch mode.
Delete input-name our driver don't distinguish pen and touch.
Will be inserted in next version.

N-trig is changing the way people interact with computers by providing a dual-mode pen and true multi-touch input device, specifically designed for today's advanced computing world.
N-trig DuoSense® solution provides a real Hands-on computing® experience, and sets the stage for OEMs and ISVs to introduce innovative computer products and applications for an intuitive, Hands-on® experience directly onscreen.
DuoSense digitizers are easily integrated into existing technologies, support all LCDs, keep devices slim and light, and can be implemented in a broad range of products, ranging from small notebooks to large LCDs.
N-trig has offices in Israel, the US, Taiwan and Japan.

Signed-off-by: Micki Balanga <micki@n-trig.com>
---
 drivers/hid/hid-ntrig.c |   41 ++++++-----------------------------------
 1 files changed, 6 insertions(+), 35 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 8cdb3c2..8bf796c 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -19,6 +19,8 @@
  *	1.3 - N-trig - Change ntrig_input_mapped
  *	1.4 - N-trig - Add ntrig_send_report function
  *		send set feature command to firmware
+ *	1.5 - N-trig udpate ntrig_probe
+ *		add call to ntrig_send_report function
  */
 
 /*
@@ -415,11 +417,6 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	int ret;
 	struct ntrig_data *nd;
-	struct hid_input *hidinput;
-	struct input_dev *input;
-
-	if (id->driver_data)
-		hdev->quirks |= HID_QUIRK_MULTI_INPUT;
 
 	nd = kmalloc(sizeof(struct ntrig_data), GFP_KERNEL);
 	if (!nd) {
@@ -427,7 +424,6 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		return -ENOMEM;
 	}
 
-	nd->reading_mt = 0;
 	hid_set_drvdata(hdev, nd);
 
 	ret = hid_parse(hdev);
@@ -442,35 +438,10 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_free;
 	}
 
-
-	list_for_each_entry(hidinput, &hdev->inputs, list) {
-		input = hidinput->input;
-		switch (hidinput->report->field[0]->application) {
-		case HID_DG_PEN:
-			input->name = "N-Trig Pen";
-			break;
-		case HID_DG_TOUCHSCREEN:
-			__clear_bit(BTN_TOOL_PEN, input->keybit);
-			/*
-			 * A little something special to enable
-			 * two and three finger taps.
-			 */
-			__set_bit(BTN_TOOL_DOUBLETAP, input->keybit);
-			__set_bit(BTN_TOOL_TRIPLETAP, input->keybit);
-			__set_bit(BTN_TOOL_QUADTAP, input->keybit);
-			/*
-			 * The physical touchscreen (single touch)
-			 * input has a value for physical, whereas
-			 * the multitouch only has logical input
-			 * fields.
-			 */
-			input->name =
-				(hidinput->report->field[0]
-				 ->physical) ?
-				"N-Trig Touchscreen" :
-				"N-Trig MultiTouch";
-			break;
-		}
+	ret = ntrig_send_report(hdev);
+	if (ret) {
+		dev_err(&hdev->dev, "send set feature failed\n");
+		goto err_free;
 	}
 
 	return 0;
-- 
1.6.3.3

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

[PATCH 6/7] HID: N-trig MTM Driver fix And cleanup patch 6

[mickib1]

From: micki <micki@micki-laptop.(none)>

Update struct hid_driver ntrig_driver. add debug module parameters

N-trig is changing the way people interact with computers by providing a dual-mode pen and true multi-touch input device, specifically designed for today's advanced computing world.
N-trig DuoSense® solution provides a real Hands-on computing® experience, and sets the stage for OEMs and ISVs to introduce innovative computer products and applications for an intuitive, Hands-on® experience directly onscreen.
DuoSense digitizers are easily integrated into existing technologies, support all LCDs, keep devices slim and light, and can be implemented in a broad range of products, ranging from small notebooks to large LCDs.
N-trig has offices in Israel, the US, Taiwan and Japan.

Signed-off-by: Micki Balanga <micki@n-trig.com>
---
 drivers/hid/hid-ntrig.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 8bf796c..f88533e 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -21,6 +21,9 @@
  *		send set feature command to firmware
  *	1.5 - N-trig udpate ntrig_probe
  *		add call to ntrig_send_report function
+ *	1.6 - N-trig Update ntrig_init print version number
+ *		update struct hid_driver
+ *		update struct hid_device_id
  */
 
 /*
@@ -458,15 +461,11 @@ static void ntrig_remove(struct hid_device *hdev)
 
 static const struct hid_device_id ntrig_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN),
-		.driver_data = NTRIG_DUPLICATE_USAGES },
+		.driver_data = NTRIG_USB_DEVICE_ID },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, ntrig_devices);
 
-static const struct hid_usage_id ntrig_grabbed_usages[] = {
-	{ HID_ANY_ID, HID_ANY_ID, HID_ANY_ID },
-	{ HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1 }
-};
 
 static struct hid_driver ntrig_driver = {
 	.name = MODULE_NAME,
@@ -475,12 +474,12 @@ static struct hid_driver ntrig_driver = {
 	.remove = ntrig_remove,
 	.input_mapping = ntrig_input_mapping,
 	.input_mapped = ntrig_input_mapped,
-	.usage_table = ntrig_grabbed_usages,
 	.event = ntrig_event,
 };
 
 static int __init ntrig_init(void)
 {
+	info("N-trig Driver Version 1.6\n");
 	return hid_register_driver(&ntrig_driver);
 }
 
-- 
1.6.3.3

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

Re: [PATCH 1/7] HID: N-trig MTM Driver fix and cleanup patch 1

[Rafi Rubin]

Just a comment before I respond to your patches.

Clearly my knowledge of the workings of your devices is limited.  I have 
no special access to your documentation or developers and no idea where 
the company plans to go in the future.

With that said, what I have seen is a set of devices which mostly follow 
standard behavior and thus are simple to support with existing drivers. 
  The kernel driver as of 2.6.33 names the device and does a tiny bit of 
clean up for pen and single touch, but mostly leaves the handling of 
those to the hid core.

Even the multi-touch support only needs slight tweaking to account for 
the "fake fingers" sent out by the hardware and some other minor points.


While I appreciate that you've put a lot of effort into writing and 
testing an in house tool stack, that doesn't justify breaking 
compatibility unnecessarily.  I too have spent considerable effort for 
more than a year (with months of non-functioning hardware before that), 
and have quite a lot of established testing time.  I do not feel that my 
work and the communal effort to test and fix justifies inhibiting your 
work, but it should justify some consideration.

Furthermore I tried to contact your company repeatedly before I began my 
work, and several times since.  That you have put so much effort in 
development and testing in isolation was your choice not mine.



So, if there is information that you can put to good use but leads to 
compatibility issues with existing tools (like the evdev driver), lets 
discuss those.

If you are ready to release the user space source, I will happily lend a 
hand synchronizing the kernel and user space.

Rafi

Re: [PATCH 1/7] HID: N-trig MTM Driver fix and cleanup patch 1

[Rafi Rubin]

On 03/08/2010 04:13 PM, mickib1@gmail.com wrote:
> From: micki<micki@micki-laptop.(none)>
>
> Add Change Log and defines of MTM firmware.
> Add debug Paramater change Driver name in hid_driver structure
>
> N-trig is changing the way people interact with computers by providing a dual-mode pen and true multi-touch input device, specifically designed for today's advanced computing world.
> N-trig DuoSense® solution provides a real Hands-on computing® experience, and sets the stage for OEMs and ISVs to introduce innovative computer products and applications for an intuitive, Hands-on® experience directly onscreen.
> DuoSense digitizers are easily integrated into existing technologies, support all LCDs, keep devices slim and light, and can be implemented in a broad range of products, ranging from small notebooks to large LCDs.
> N-trig has offices in Israel, the US, Taiwan and Japan.

Is that just in your .sig?

> Signed-off-by: Micki Balanga<micki@n-trig.com>
> ---
>   drivers/hid/hid-ntrig.c |   94 ++++++++++++++++++++++++++++++++++++++++++++--
>   1 files changed, 89 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
> index 3234c72..e99342d 100644
> --- a/drivers/hid/hid-ntrig.c
> +++ b/drivers/hid/hid-ntrig.c
> @@ -3,7 +3,18 @@
>    *
>    *  Copyright (c) 2008 Rafi Rubin
>    *  Copyright (c) 2009 Stephane Chatty
> + *  Copyright (c) 2010 N-TRIG
>    *
> + *	1.0 - Rafi Rubin ( Version From 2010-02-13 02:13:05)
> + *		This cleans up the identification of multitouch
> + *		groups and enables the end of group sync.
> + *		Taps are now explicitly handled to adjust for the changes in the
> + *		event stream in multitouch mode.
> + *		Added triple and quad tap for the benefit of
> + *		tools that recognize different tap types but
> + *		do not have full multi touch support.
> + *	1.1 - N-trig - Add Change Log and defines of MTM firmware.
> + *		Add debug Paramater change Driver name in hid_driver structure
>    */

I don't know that the change log needs to be in the code file itself. 
The point was more that your commit messages should give us an idea 
about what your changing.  Jiri, what's your take on that?

> +/*
> + * Pen Event Field Declaration
> + */
> +#define	EVENT_PEN_TIP		0x03
> +#define	EVENT_PEN_RIGHT		0x05
> +#define	EVENT_TOUCH_PEN		0x07
> +#define EVENT_PEN_IN_RANGE	0x01

You still haven't actually established why its necessary to fuss with 
pen events if we split the pen off to a separate input device.

> +/*
> + * MTM 4 last bytes of report descriptor
> + */
> +#define REPORT_GENERIC1		0x01
> +#define REPORT_MT		0x02
> +#define REPORT_PALM		0x03
> +#define REPORT_GENERIC2		0x04

When its not too inconvenient, it would be better to put defines in the 
patches where you actually use them.  I know these are actually useful, 
but if we stop here they aren't actually used.


> +#define NTRIG_USB_DEVICE_ID	0x0001
Already defined in hid-ids.h



> +
> +/*
> + * MTM fields
> + */
> +#define PEN_REPORT_SIZE		0x48

The report seems to specifically identify itself.  Why not use that 
clear identification instead of something less clear that may change in 
the future (even if there are no plans at the moment).  If nothing else 
this relieves a little pressure on your hardware/firmware designers.

 From the rdesc of my screen in the debug fs:
INPUT(1)[INPUT]
     Field(0)
       Physical(Digitizers.Stylus)

Paraphrasing ntrig_probe from 2.6.33:

   list_for_each_entry(hidinput, &hdev->inputs, list)
	switch (hidinput->report->field[0]->application)
	case  HID_DG_PEN:
		input->name = "N-Trig Pen";
		break;
	case HID_DG_TOUCHSCREEN:

Note that in this one case I do use this to identify the report, and 
thus need to pick a field which you could argue is a potential hazard, 
but you haven't made that point.  So please tell us what's wrong with 
this before you toss it out for something different.

> +#define MAX_FINGERS_SUPPORT	0x06

Again, is it really a good idea to keep this quantity fixed?  Especially 
in light of the press release that suggests your company is considering 
devices that support more fingers.

> +#define END_OF_REPORT		0x64

I should comment where you put the code that uses this, but you put them 
in separate patches.

I don't see why you tie up a group of contacts by emitting TRACKING_ID 
0x64.  The group should already by wrapped up with an input_sync, which 
you have.  Therefore this extra event doesn't actually provide any 
additional information.  Furthermore, sending a bogus tracking id may 
upset other user space tools.

> +/*
> + * Dummy Finger Declaration
> + */
> +#define X_CORD_VAL		0x00
> +#define Y_CORD_VAL		0x00
> +#define DX_CORD_VAL		0xFA
> +#define DY_CORD_VAL		0x96
> +#define GENERIC_BYTE_VAL	0x0D

The naming of these constants seems a little messy.  Perhaps something 
more like "DUMMY_X, DUMMY_DX"....

> +#define MTM_FRAME_INDEX		0xff000001
> +#define MTM_PROPROETARY		0xff000002

Thank you, we should have named these constants earlier.

> +#define MODULE_NAME "hid_ntrig"
> -	.name = "ntrig",

I left off the "hid_" prefix based on what I saw in the other hid 
drivers.  Jiri, is there a standard to follow or do you have an opinion 
about this?

Rafi

Re: [PATCH 2/7] HID: N-trig MTM Driver fix and cleanup patch 2

[Rafi Rubin]

> -		case HID_DG_CONTACTID: /* Not trustworthy, squelch for now */

I squelched contact id because they don't actually track fingers, rather 
contacts are reported spatially (seems to be from the bottom to top) 
making this value useless for tracking (after all the user space tools 
can count contacts since the start of the group).

See the "Finger Tracking" section in
	Documentation/input/multi-touch-protocol.txt
When tracking id is incorrect its better to just leave it out.

> +		case HID_DG_INVERT: /* value is always 0 */
> +		case HID_DG_ERASER: /* value is always 0 */
> +		case HID_DG_CONTACTID: /* value is useless */
> +		case HID_DG_BARRELSWITCH:  /* doubtful */

Stephane put these here erroneously.  He was working with a firmware 
that didn't support the stylus at all when he made those observations. 
These events are useful and should not be squelched.

> +		case HID_DG_CONTACTCOUNT:

See the rdesc for the device.
"Digitizers.ContactCount ---> Sync.Report"

As the last event sent by a mt group, that is already mapped to 
input_sync().  If you don't squelch it, you don't need to implicitly add 
the sync.

>   		case HID_DG_HEIGHT:
>   			hid_map_usage(hi, usage, bit, max,
>   					EV_ABS, ABS_MT_TOUCH_MINOR);
> -			input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
> -					0, 1, 0, 0);

Stephane added the orientation to height, I can't comment about the 
wisdom of pulling it.

Rafi

Re: [PATCH 4/7] HID: N-trig MTM Driver fix And cleanup Patch 4

[Rafi Rubin]

Very interesting.

Minor side point.  This would be the patch to add the usb includes.

Rafi

On 03/08/2010 04:13 PM, mickib1@gmail.com wrote:
> From: micki<micki@micki-laptop.(none)>
>
> Add ntrig_send_report - enable us to configure firmware.
>
> N-trig is changing the way people interact with computers by providing a dual-mode pen and true multi-touch input device, specifically designed for today's advanced computing world.
> N-trig DuoSense® solution provides a real Hands-on computing® experience, and sets the stage for OEMs and ISVs to introduce innovative computer products and applications for an intuitive, Hands-on® experience directly onscreen.
> DuoSense digitizers are easily integrated into existing technologies, support all LCDs, keep devices slim and light, and can be implemented in a broad range of products, ranging from small notebooks to large LCDs.
> N-trig has offices in Israel, the US, Taiwan and Japan.
>
> Signed-off-by: Micki Balanga<micki@n-trig.com>
> ---
>   drivers/hid/hid-ntrig.c |   50 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 50 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
> index 8130c3c..8cdb3c2 100644
> --- a/drivers/hid/hid-ntrig.c
> +++ b/drivers/hid/hid-ntrig.c
> @@ -17,6 +17,8 @@
>    *		Add debug Paramater change Driver name in hid_driver structure
>    *	1.2 - N-trig - Change ntrig_input_mapping
>    *	1.3 - N-trig - Change ntrig_input_mapped
> + *	1.4 - N-trig - Add ntrig_send_report function
> + *		send set feature command to firmware
>    */
>
>   /*
> @@ -361,6 +363,54 @@ static int ntrig_event(struct hid_device *hid, struct hid_field *field,
>   	return 1;
>   }
>
> +/*
> + * This function used to configure N-trig firmware
> + * The first command we need to send to firmware is change
> + * to Multi-touch Mode we don't receive a reply
> + */
> +static int ntrig_send_report(struct hid_device *hid)
> +{
> +	struct hid_report *report;
> +	struct list_head *report_list =
> +			&hid->report_enum[HID_FEATURE_REPORT].report_list;
> +
> +	if (list_empty(report_list)) {
> +		ntrig_dbg("no feature reports found\n");
> +		return -ENODEV;
> +	}
> +	report = list_first_entry(report_list, struct hid_report, list);
> +	if (report->maxfield<  1)
> +		return -ENODEV;
> +
> +	list_for_each_entry(report,
> +			    report_list, list) {
> +		if (report->maxfield<  1) {
> +		      ntrig_dbg("no fields in the report\n");
> +		      continue;
> +		}
> +		ntrig_dbg("Report ID %x\n", report->id);
> +		switch (report->id) {
> +		case REPORTID_DRIVER_ALIVE:
> +		      usbhid_submit_report(hid, report, USB_DIR_OUT);
> +		      break;
> +		      /*
> +		       * These command will implement later accroding to demand
> +		       */
> +		case REPORTID_GET_VERSION:	/* FALLTHRU */
> +		case REPORTID_SET_MODE_DUAL:	/* FALLTHRU */
> +		case REPORTID_CALIBRATION:	/* FALLTHRU */
> +		case REPORTID_GET_MODE:		/* FALLTHRU */
> +		case REPORTID_SET_MODE_PEN:	/* FALLTHRU */
> +		case REPORTID_SET_MODE_TOUCH:	/* FALLTHRU */
> +		case REPORTID_CALIBRATION_RESPOND:/* FALLTHRU */
> +		case HID_CAPACITORS_CALIB:	/* FALLTHRU */
> +		case HID_GET_CAPACITORS_CALIB_DONE:
> +		      break;
> +		}
> +	}
> +	return 0;
> +}
> +
>   static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
>   {
>   	int ret;

Re: [PATCH 6/7] HID: N-trig MTM Driver fix And cleanup patch 6

[Rafi Rubin]

>   static const struct hid_device_id ntrig_devices[] = {
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN),
> -		.driver_data = NTRIG_DUPLICATE_USAGES },
> +		.driver_data = NTRIG_USB_DEVICE_ID },

The vendor and device id are in the first field "HID_USB_DEVICE", driver 
data is for flags, not id.  Its merely a coincidence that they are the 
same in this case.

The duplicate usages tells hid to let you have more than one "x" and "y" 
axis.  Otherwise each finger gets a new set (the second finger gets "z" 
and "rx" and so on).

>   static int __init ntrig_init(void)
>   {
> +	info("N-trig Driver Version 1.6\n");

Perhaps you mean:
MODULE_VERSION("1.06");

Re: [PATCH 5/7] HID: N-trig MTM Driver fix And cleanup patch 5

[Rafi Rubin]

> -	if (id->driver_data)
> -		hdev->quirks |= HID_QUIRK_MULTI_INPUT;

This undoes the unfortunate merger of the pen and touch events.  Why 
would you want to remove that?  Is there any actual reason aside from 
that's the way you've done it so far?

I admit that if the user space is smart enough to distinguish the 
multiplexed streams, its not that big a deal.  But even then it 
complicates the user space and adds a few assumptions.

Also, splitting them gives the user the freedom to use different drivers 
for the different sensors.  I understand why you'd want to encourage 
users to use your stack for everything, but that also means you are 
potentially increasing your own support burden.

All in all, it just seems like a bad idea to me.
> -	list_for_each_entry(hidinput,&hdev->inputs, list) {
> -		input = hidinput->input;
> -		switch (hidinput->report->field[0]->application) {
> -		case HID_DG_PEN:
> -			input->name = "N-Trig Pen";
> -			break;
> -		case HID_DG_TOUCHSCREEN:
> -			__clear_bit(BTN_TOOL_PEN, input->keybit);
> -			/*
> -			 * A little something special to enable
> -			 * two and three finger taps.
> -			 */
> -			__set_bit(BTN_TOOL_DOUBLETAP, input->keybit);
> -			__set_bit(BTN_TOOL_TRIPLETAP, input->keybit);
> -			__set_bit(BTN_TOOL_QUADTAP, input->keybit);
> -			/*
> -			 * The physical touchscreen (single touch)
> -			 * input has a value for physical, whereas
> -			 * the multitouch only has logical input
> -			 * fields.
> -			 */
> -			input->name =
> -				(hidinput->report->field[0]
> -				 ->physical) ?
> -				"N-Trig Touchscreen" :
> -				"N-Trig MultiTouch";
> -			break;
> -		}

Even if you want to argue that you need to pull the multi-input quirk, 
that doesn't justify pulling the names.  Perhaps you may wish to 
truncate the name to "N-Trig" or something.  But give them some name. 
Those names simplify driver correlation in user space.

Rafi

Re: [PATCH 1/7] HID: N-trig MTM Driver fix and cleanup patch 1

[Dmitry Torokhov]

On Mon, Mar 08, 2010 at 11:13:41PM +0200, mickib1@gmail.com wrote:
> From: micki <micki@micki-laptop.(none)>
> 
> Add Change Log and defines of MTM firmware.
> Add debug Paramater change Driver name in hid_driver structure
> 

---------------------------------------

> N-trig is changing the way people interact with computers by providing a dual-mode pen and true multi-touch input device, specifically designed for today's advanced computing world.
> N-trig DuoSense® solution provides a real Hands-on computing® experience, and sets the stage for OEMs and ISVs to introduce innovative computer products and applications for an intuitive, Hands-on® experience directly onscreen.
> DuoSense digitizers are easily integrated into existing technologies, support all LCDs, keep devices slim and light, and can be implemented in a broad range of products, ranging from small notebooks to large LCDs.
> N-trig has offices in Israel, the US, Taiwan and Japan.
> 

----------------------------------------

The above certainly does not belong to kernel changelog.

> Signed-off-by: Micki Balanga <micki@n-trig.com>
> ---
>  drivers/hid/hid-ntrig.c |   94 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 89 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
> index 3234c72..e99342d 100644
> --- a/drivers/hid/hid-ntrig.c
> +++ b/drivers/hid/hid-ntrig.c
> @@ -3,7 +3,18 @@
>   *
>   *  Copyright (c) 2008 Rafi Rubin
>   *  Copyright (c) 2009 Stephane Chatty
> + *  Copyright (c) 2010 N-TRIG
>   *
> + *	1.0 - Rafi Rubin ( Version From 2010-02-13 02:13:05)
> + *		This cleans up the identification of multitouch
> + *		groups and enables the end of group sync.
> + *		Taps are now explicitly handled to adjust for the changes in the
> + *		event stream in multitouch mode.
> + *		Added triple and quad tap for the benefit of
> + *		tools that recognize different tap types but
> + *		do not have full multi touch support.
> + *	1.1 - N-trig - Add Change Log and defines of MTM firmware.
> + *		Add debug Paramater change Driver name in hid_driver structure

No need to keep changelog in the file, we have scm for a reason.

Also, all patches split in a wied fashion, being not really useable on
their own. For exapmple thi spatch adds ntrig_dbg but no users for this
facility.

-- 
Dmitry
--
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

Re: [PATCH 1/7] HID: N-trig MTM Driver fix and cleanup patch 1

[Jiri Kosina]

On Mon, 8 Mar 2010, Rafi Rubin wrote:

> > + *	1.0 - Rafi Rubin ( Version From 2010-02-13 02:13:05)
> > + *		This cleans up the identification of multitouch
> > + *		groups and enables the end of group sync.
> > + *		Taps are now explicitly handled to adjust for the changes in
> > the
> > + *		event stream in multitouch mode.
> > + *		Added triple and quad tap for the benefit of
> > + *		tools that recognize different tap types but
> > + *		do not have full multi touch support.
> > + *	1.1 - N-trig - Add Change Log and defines of MTM firmware.
> > + *		Add debug Paramater change Driver name in hid_driver structure
> >    */
> 
> I don't know that the change log needs to be in the code file itself. The
> point was more that your commit messages should give us an idea about what
> your changing.  Jiri, what's your take on that?

Fully agreed. I am all for removing any Changelog entries from the .c 
source files altogether. We have excellent SCM to handle this all for us.

> Thank you, we should have named these constants earlier.
> 
> > +#define MODULE_NAME "hid_ntrig"
> > -	.name = "ntrig",
> 
> I left off the "hid_" prefix based on what I saw in the other hid drivers.
> Jiri, is there a standard to follow or do you have an opinion about this?

The convention used by all the other drivers hanging on HID bus are 
without the 'hid' prefix, so we should obey that also for ntrig, yes.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 6/7] HID: N-trig MTM Driver fix And cleanup patch 6

[Jiri Kosina]

On Mon, 8 Mar 2010, Rafi Rubin wrote:

> >   static int __init ntrig_init(void)
> >   {
> > +	info("N-trig Driver Version 1.6\n");
> 
> Perhaps you mean:
> MODULE_VERSION("1.06");

I'd rather remove all the MODULE_VERSION stuff altogether. git is much 
better tracking revisions, once the driver gets merged.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 2/7] HID: N-trig MTM Driver fix and cleanup patch 2

[Stéphane Chatty]

Le 9 mars 10 à 02:07, Rafi Rubin a écrit :
>
>>   		case HID_DG_HEIGHT:
>>   			hid_map_usage(hi, usage, bit, max,
>>   					EV_ABS, ABS_MT_TOUCH_MINOR);
>> -			input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
>> -					0, 1, 0, 0);
>
> Stephane added the orientation to height, I can't comment about the  
> wisdom of pulling it.

The MT kernel protocol says that width/height is reported through  
TouchMinor/TouchMajor/Orientation. Orientation needs to be added  
somewhere. Doing it when encountering width or height seemed a  
logical thing to do, but I would see no objection at doing it  
somewhere else with TouchMinor and TouchMajor.

Cheers,

St.

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