Hi Stephane,

On Fri, Dec 11, 2009 at 03:10:38PM +0100, Stephane Chatty wrote:

I thought we agreed that this option is unnecessary.

+struct mmm_finger {
+	__s32 x, y;
+	__u8 rank;
+	int touch:1, valid:1;
+};

Does it make sense to turn access to touch and valid into
read-modify-write sequence?  Just change them to be 'bool's, it won't
cause your structures to grow in size.
 

+struct mmm_data {
+	struct mmm_finger f[10];
+	__u8 curid, num;
+	int touch:1, valid:1;

Same as above.

+};
+
+static int mmm_input_mapping(struct hid_device *hdev, struct hid_input *hi,
+		struct hid_field *field, struct hid_usage *usage,
+		unsigned long **bit, int *max)
+{
+	switch (usage->hid & HID_USAGE_PAGE) {
+
+	case HID_UP_BUTTON:
+		return -1;
+
+	case HID_UP_GENDESK:
+		switch (usage->hid) {
+		case HID_GD_X:
+			hid_map_usage(hi, usage, bit, max,
+					EV_ABS, ABS_MT_POSITION_X);
+			if (emulate_touchscreen)
+				input_set_abs_params(hi->input, ABS_X,
+						field->logical_minimum,
+						field->logical_maximum, 0, 0);
+			return 1;
+		case HID_GD_Y:
+			hid_map_usage(hi, usage, bit, max,
+					EV_ABS, ABS_MT_POSITION_Y);
+			if (emulate_touchscreen)
+				input_set_abs_params(hi->input, ABS_Y,
+						field->logical_minimum,
+						field->logical_maximum, 0, 0);
+			return 1;
+		}
+		return 0;
+
+	case HID_UP_DIGITIZER:
+		switch (usage->hid) {
+		/* we do not want to map these: no input-oriented meaning */
+		case 0x14:
+		case 0x23:
+		case HID_DG_INPUTMODE:
+		case HID_DG_DEVICEINDEX:
+		case HID_DG_CONTACTCOUNT:
+		case HID_DG_CONTACTMAX:
+		case HID_DG_INRANGE:
+		case HID_DG_CONFIDENCE:
+			return -1;
+		case HID_DG_TIPSWITCH:
+			if (emulate_touchscreen) {
+				hid_map_usage(hi, usage, bit, max,
+					EV_KEY, BTN_TOUCH);
+				return 1;
+			} else
+				return -1;
+		case HID_DG_CONTACTID:
+			hid_map_usage(hi, usage, bit, max,
+					EV_ABS, ABS_MT_TRACKING_ID);
+			return 1;
+		}
+		/* let hid-input decide for the others */
+		return 0;
+
+	case 0xff000000:
+		/* we do not want to map these: no input-oriented meaning */
+		return -1;
+	}
+
+	return 0;
+}
+
+static int mmm_input_mapped(struct hid_device *hdev, struct hid_input *hi,
+		struct hid_field *field, struct hid_usage *usage,
+		unsigned long **bit, int *max)
+{
+	if (usage->type == EV_KEY || usage->type == EV_ABS)
+		clear_bit(usage->code, *bit);
+
+	return 0;
+}
+
+/*
+ * this function is called when a whole packet has been received and process,
+ * so that it can decide what to send to the input layer.
+ */
+static void mmm_filter_event(struct mmm_data *md, struct input_dev *input)
+{
+	struct mmm_finger *oldest = 0;
+	int pressed = 0, released = 0;

These are bools so make them so.

+	int i;

Blank lines between declarations and code are appreciated.

+	for (i = 0; i < 10; ++i) {
+		struct mmm_finger *f = &md->f[i];
+		if (!f->valid) {
+			/* ignore placeholder data */
+		} else if (f->touch) {
+			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, i);
+			input_event(input, EV_ABS, ABS_MT_POSITION_X, f->x);
+			input_event(input, EV_ABS, ABS_MT_POSITION_Y, f->y);
+			input_mt_sync(input);
+			/*
+			 * maintain age rank of this finger for our single
+			 * touch emulation.
+			 */
+			if (f->rank == 0) {
+				f->rank = ++(md->num);
+				if (f->rank == 1)
+					pressed = 1;
+			}
+			if (f->rank == 1)
+				oldest = f;
+		} else {
+			/*
+			 * maintain age rank of other fingers for our single
+			 * touch emulation.
+			 */
+			int j;
+			for (j = 0; j < 10; ++j) {
+				struct mmm_finger *g = &md->f[j];
+				if (g->rank > f->rank) {
+					g->rank--;
+					if (g->rank == 1)
+						oldest = g;
+				}
+			}
+			f->rank = 0;
+			--(md->num);
+			if (md->num == 0)
+				released = 1;
+		}

Maybe move out into a separate function to report one finger?

+		f->valid = 0;
+	}
+
+	if (emulate_touchscreen) {
+		if (oldest) {
+			if (pressed)
+				input_event(input, EV_KEY, BTN_TOUCH, 1);
+			input_event(input, EV_ABS, ABS_X, oldest->x);
+			input_event(input, EV_ABS, ABS_Y, oldest->y);
+		} else if (released) {
+			input_event(input, EV_KEY, BTN_TOUCH, 0);
+		}
+	}
+}
+
+/*
+ * this function is called upon all reports
+ * so that we can accumulate contact point information,
+ * and call input_mt_sync after each point.
+ */
+static int mmm_event(struct hid_device *hid, struct hid_field *field,
+			struct hid_usage *usage, __s32 value)
+{
+	struct mmm_data *md = hid_get_drvdata(hid);
+       /*

Indent with tab please.

+	 * Strangely, this function can be called before
+	 * field->hidinput is initialized!
+	 */
+
+	if (hid->claimed & HID_CLAIMED_INPUT) {
+		struct input_dev *input = field->hidinput->input;
+		switch (usage->hid) {
+		case HID_DG_TIPSWITCH:
+			md->touch = value;
+			break;
+		case HID_DG_CONFIDENCE:
+			md->valid = value;
+			break;
+		case HID_DG_CONTACTID:
+			if (md->valid) {
+				md->curid = value;
+				md->f[value].touch = md->touch;
+				md->f[value].valid = 1;
+			}
+			break;
+		case HID_GD_X:
+			if (md->valid)
+				md->f[md->curid].x = value;
+			break;
+		case HID_GD_Y:
+			if (md->valid)
+				md->f[md->curid].y = value;
+			break;
+		case HID_DG_CONTACTCOUNT:
+			mmm_filter_event(md, input);
+			break;
+		}
+	}
+
+	/* we have handled the hidinput part, now remains hiddev */
+	if (hid->claimed & HID_CLAIMED_HIDDEV && hid->hiddev_hid_event)
+		hid->hiddev_hid_event(hid, field, usage, value);
+
+	return 1;
+}
+
+static int mmm_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+	int ret;
+	struct mmm_data *md;
+
+	md = kzalloc(sizeof(struct mmm_data), GFP_KERNEL);
+	if (!md) {
+		dev_err(&hdev->dev, "cannot allocate 3M data\n");
+		return -ENOMEM;
+	}
+	hid_set_drvdata(hdev, md);
+
+	ret = hid_parse(hdev);
+	if (!ret)
+		ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+
+	if (ret)
+		kfree(md);
+	return ret;
+}
+
+static void mmm_remove(struct hid_device *hdev)
+{
+	hid_hw_stop(hdev);
+	kfree(hid_get_drvdata(hdev));

hid_set_drvdata(hdev, NULL); is a polite thing to do.

+}
+
+static const struct hid_device_id mmm_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_3M, USB_DEVICE_ID_3M1968) },
+	{ }
+};
+MODULE_DEVICE_TABLE(hid, mmm_devices);
+
+static const struct hid_usage_id mmm_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 mmm_driver = {
+	.name = "3m-pct",
+	.id_table = mmm_devices,
+	.probe = mmm_probe,
+	.remove = mmm_remove,
+	.input_mapping = mmm_input_mapping,
+	.input_mapped = mmm_input_mapped,
+	.usage_table = mmm_grabbed_usages,
+	.event = mmm_event,
+};
+
+static int mmm_init(void)

__init

+{
+	return hid_register_driver(&mmm_driver);
+}
+
+static void mmm_exit(void)

__exit

Also please add Jiri Kosina [off-list ref] to your next submission
since the driver will go through his tree. Thanks.

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

-- 
Dmitry