On Tue, 19 May 2009, Stéphane Chatty wrote:

+/*
+   this driver is aimed at two firmware versions in circulation:
+    - dual pen/finger single touch
+    - finger multitouch, pen not working
+*/

Please use kernel-style comments (i.e. stars on each line).

+	case 0xff000000:
+		/* we do not want to map these */

Why?

+/*
+ * this function is called upon all reports
+ * so that we can filter contact point information,
+ * 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 __s32 x, y, id, w, h;
+	static char reading_a_point = 0, found_contact_id = 0;
+	struct input_dev *input = field->hidinput->input;

Why are these variables static? This will break horribly if multiple 
devices are connected to the system, right?

+		default:
+                	hidinput_hid_event(hid, field, usage, value);

Just return 1; here?

+		}
+	}
+        if (hid->claimed & HID_CLAIMED_HIDDEV && hid->hiddev_hid_event)
+                hid->hiddev_hid_event(hid, field, usage, value);
+
+	return 1;

And return 0; here and let the HID core do all the rest (passing to 
hidinput and hiddev, etc) if necessary?

Thanks,

-- 
Jiri Kosina
SUSE Labs