[PATCH 0/6] hid: egalax: Rework to include joojoo support

[PATCH 0/6] hid: egalax: Rework to include joojoo support

[Henrik Rydberg]

Hi Jiri, Stephane,

This series adds support for the joojoo tablet. Based on the
jikos/egalax branch, the first two patches prepare for the change to
MT slots. The third patch fixes the bug that was also fixed by commit
f516611, but in another (agreed) way. The fourth patch fixes another
minor problem. The fifth patch adds event filtering based on observed
signal quality, and the sixth patch does the heavy lifting of
converting to MT slots and thereby solving the major problem for the
joojoo.

The patches have been tested on both supported devices. Thanks Philipp!

Henrik

Henrik Rydberg (6):
  hid: egalax: Use kzalloc
  hid: egalax: Setup input device manually
  hid: egalax: Correct for device resolution report error
  hid: egalax: Report zero as minimum pressure
  hid: egalax: Add event filtering
  hid: egalax: Convert to MT slots

 drivers/hid/hid-egalax.c |  177 +++++++++++++++++++++++++++-------------------
 1 files changed, 103 insertions(+), 74 deletions(-)

[PATCH 1/6] hid: egalax: Use kzalloc

[Henrik Rydberg]

To avoid unnecessary explicit initialization, allocate zeroed memory.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/hid/hid-egalax.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
index 54b017a..5a1b52e 100644
--- a/drivers/hid/hid-egalax.c
+++ b/drivers/hid/hid-egalax.c
@@ -221,7 +221,7 @@ static int egalax_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	struct egalax_data *td;
 	struct hid_report *report;
 
-	td = kmalloc(sizeof(struct egalax_data), GFP_KERNEL);
+	td = kzalloc(sizeof(struct egalax_data), GFP_KERNEL);
 	if (!td) {
 		dev_err(&hdev->dev, "cannot allocate eGalax data\n");
 		return -ENOMEM;
-- 
1.7.1

[PATCH 2/6] hid: egalax: Setup input device manually

[Henrik Rydberg]

The hid core does not yet handle input filtering. Take over the setup
of the input device, so that proper signal-to-noise ratios can be
used.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/hid/hid-egalax.c |   37 +++++++++++++++++++++++++------------
 1 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
index 5a1b52e..23f832b 100644
--- a/drivers/hid/hid-egalax.c
+++ b/drivers/hid/hid-egalax.c
@@ -25,6 +25,8 @@ MODULE_LICENSE("GPL");
 
 #include "hid-ids.h"
 
+#define MAX_EVENTS		120
+
 struct egalax_data {
 	__u16 x, y, z;
 	__u8 id;
@@ -34,10 +36,21 @@ struct egalax_data {
 	__u16 lastx, lasty, lastz;	/* latest valid (x, y, z) in the frame */
 };
 
+static void set_abs(struct input_dev *input, unsigned int code,
+		    struct hid_field *field, int snratio)
+{
+	int fmin = field->logical_minimum;
+	int fmax = field->logical_maximum;
+	int fuzz = snratio ? (fmax - fmin) / snratio : 0;
+	input_set_abs_params(input, code, fmin, fmax, fuzz, 0);
+}
+
 static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
+	struct input_dev *input = hi->input;
+
 	switch (usage->hid & HID_USAGE_PAGE) {
 
 	case HID_UP_GENDESK:
@@ -45,18 +58,16 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		case HID_GD_X:
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_POSITION_X);
+			set_abs(input, ABS_MT_POSITION_X, field, 0);
 			/* touchscreen emulation */
-			input_set_abs_params(hi->input, ABS_X,
-						field->logical_minimum,
-						field->logical_maximum, 0, 0);
+			set_abs(input, ABS_X, field, 0);
 			return 1;
 		case HID_GD_Y:
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_POSITION_Y);
+			set_abs(input, ABS_MT_POSITION_Y, field, 0);
 			/* touchscreen emulation */
-			input_set_abs_params(hi->input, ABS_Y,
-						field->logical_minimum,
-						field->logical_maximum, 0, 0);
+			set_abs(input, ABS_Y, field, 0);
 			return 1;
 		}
 		return 0;
@@ -66,6 +77,7 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		case HID_DG_TIPSWITCH:
 			/* touchscreen emulation */
 			hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
+			input_set_capability(input, EV_KEY, BTN_TOUCH);
 			return 1;
 		case HID_DG_INRANGE:
 		case HID_DG_CONFIDENCE:
@@ -75,14 +87,15 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		case HID_DG_CONTACTID:
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_TRACKING_ID);
+			set_abs(input, ABS_MT_TRACKING_ID, field, 0);
+			input_set_events_per_packet(input, MAX_EVENTS);
 			return 1;
 		case HID_DG_TIPPRESSURE:
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_PRESSURE);
+			set_abs(input, ABS_MT_PRESSURE, field, 0);
 			/* touchscreen emulation */
-			input_set_abs_params(hi->input, ABS_PRESSURE,
-						field->logical_minimum,
-						field->logical_maximum, 0, 0);
+			set_abs(input, ABS_PRESSURE, field, 0);
 			return 1;
 		}
 		return 0;
@@ -96,10 +109,10 @@ static int egalax_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
+	/* tell hid-input to skip setup of these event types */
 	if (usage->type == EV_KEY || usage->type == EV_ABS)
-		clear_bit(usage->code, *bit);
-
-	return 0;
+		set_bit(usage->type, hi->input->evbit);
+	return -1;
 }
 
 /*
-- 
1.7.1

[PATCH 3/6] hid: egalax: Correct for device resolution report error

[Henrik Rydberg]

The firmware of both supported devices report a X/Y maximum of 4096,
whereas in reality, it is eight times larger. Fixed with this patch.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/hid/hid-egalax.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
index 23f832b..3b490a3 100644
--- a/drivers/hid/hid-egalax.c
+++ b/drivers/hid/hid-egalax.c
@@ -56,6 +56,7 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	case HID_UP_GENDESK:
 		switch (usage->hid) {
 		case HID_GD_X:
+			field->logical_maximum <<= 3;
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_POSITION_X);
 			set_abs(input, ABS_MT_POSITION_X, field, 0);
@@ -63,6 +64,7 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			set_abs(input, ABS_X, field, 0);
 			return 1;
 		case HID_GD_Y:
+			field->logical_maximum <<= 3;
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_POSITION_Y);
 			set_abs(input, ABS_MT_POSITION_Y, field, 0);
-- 
1.7.1

[PATCH 4/6] hid: egalax: Report zero as minimum pressure

[Henrik Rydberg]

The firmware reports a logical minimum of one, but in order for
userspace applications to correctly map all reported values to
non-zero pressure, the driver needs to report a logical minimum of
zero.  Fixed with this patch.

Tested-by: Philipp Merkel <mail@philmerk.de>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/hid/hid-egalax.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
index 3b490a3..2aba897 100644
--- a/drivers/hid/hid-egalax.c
+++ b/drivers/hid/hid-egalax.c
@@ -93,6 +93,7 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			input_set_events_per_packet(input, MAX_EVENTS);
 			return 1;
 		case HID_DG_TIPPRESSURE:
+			field->logical_minimum = 0;
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_PRESSURE);
 			set_abs(input, ABS_MT_PRESSURE, field, 0);
-- 
1.7.1

[PATCH 5/6] hid: egalax: Add event filtering

[Henrik Rydberg]

Use estimated signal-to-noise ratios to reduce noise and limit the
amount of events emitted.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/hid/hid-egalax.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
index 2aba897..b21e7dc 100644
--- a/drivers/hid/hid-egalax.c
+++ b/drivers/hid/hid-egalax.c
@@ -27,6 +27,10 @@ MODULE_LICENSE("GPL");
 
 #define MAX_EVENTS		120
 
+/* estimated signal-to-noise ratios */
+#define SN_MOVE			1024
+#define SN_PRESSURE		32
+
 struct egalax_data {
 	__u16 x, y, z;
 	__u8 id;
@@ -59,17 +63,17 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			field->logical_maximum <<= 3;
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_POSITION_X);
-			set_abs(input, ABS_MT_POSITION_X, field, 0);
+			set_abs(input, ABS_MT_POSITION_X, field, SN_MOVE);
 			/* touchscreen emulation */
-			set_abs(input, ABS_X, field, 0);
+			set_abs(input, ABS_X, field, SN_MOVE);
 			return 1;
 		case HID_GD_Y:
 			field->logical_maximum <<= 3;
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_POSITION_Y);
-			set_abs(input, ABS_MT_POSITION_Y, field, 0);
+			set_abs(input, ABS_MT_POSITION_Y, field, SN_MOVE);
 			/* touchscreen emulation */
-			set_abs(input, ABS_Y, field, 0);
+			set_abs(input, ABS_Y, field, SN_MOVE);
 			return 1;
 		}
 		return 0;
@@ -96,9 +100,9 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			field->logical_minimum = 0;
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_PRESSURE);
-			set_abs(input, ABS_MT_PRESSURE, field, 0);
+			set_abs(input, ABS_MT_PRESSURE, field, SN_PRESSURE);
 			/* touchscreen emulation */
-			set_abs(input, ABS_PRESSURE, field, 0);
+			set_abs(input, ABS_PRESSURE, field, SN_PRESSURE);
 			return 1;
 		}
 		return 0;
-- 
1.7.1

[PATCH 6/6] hid: egalax: Convert to MT slots

[Henrik Rydberg]

The joojoo reports touches sequentially, one per report, which
confuses the current driver. Convert to the MT slots protocol and use
the stored slot information to emulate pointer movement in a stable
manner.

Tested-by: Philipp Merkel <mail@philmerk.de>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/hid/hid-egalax.c |  133 ++++++++++++++++++++++++---------------------
 1 files changed, 71 insertions(+), 62 deletions(-)

diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
index b21e7dc..1ca0f84 100644
--- a/drivers/hid/hid-egalax.c
+++ b/drivers/hid/hid-egalax.c
@@ -2,6 +2,8 @@
  *  HID driver for eGalax dual-touch panels
  *
  *  Copyright (c) 2010 Stephane Chatty <chatty@enac.fr>
+ *  Copyright (c) 2010 Henrik Rydberg <rydberg@euromail.se>
+ *  Copyright (c) 2010 Canonical, Ltd.
  *
  */
 
@@ -25,19 +27,25 @@ MODULE_LICENSE("GPL");
 
 #include "hid-ids.h"
 
+#define MAX_SLOTS		2
+#define MAX_TRKID		USHRT_MAX
 #define MAX_EVENTS		120
 
 /* estimated signal-to-noise ratios */
 #define SN_MOVE			1024
 #define SN_PRESSURE		32
 
+struct egalax_contact {
+	int touch;
+	int x, y, z;
+};
+
 struct egalax_data {
-	__u16 x, y, z;
-	__u8 id;
-	bool first;		/* is this the first finger in the frame? */
-	bool valid;		/* valid finger data, or just placeholder? */
-	bool activity;		/* at least one active finger previously? */
-	__u16 lastx, lasty, lastz;	/* latest valid (x, y, z) in the frame */
+	struct egalax_contact contact[MAX_SLOTS];
+	struct egalax_contact single, tmp;
+	int valid;
+	int slot;
+	int trkid;
 };
 
 static void set_abs(struct input_dev *input, unsigned int code,
@@ -91,10 +99,13 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		case HID_DG_CONTACTMAX:
 			return -1;
 		case HID_DG_CONTACTID:
+			field->logical_maximum = MAX_TRKID;
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_TRACKING_ID);
 			set_abs(input, ABS_MT_TRACKING_ID, field, 0);
 			input_set_events_per_packet(input, MAX_EVENTS);
+			if (!input->mt)
+				input_mt_create_slots(input, MAX_SLOTS);
 			return 1;
 		case HID_DG_TIPPRESSURE:
 			field->logical_minimum = 0;
@@ -122,64 +133,61 @@ static int egalax_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 	return -1;
 }
 
+static void emulate_pointer(struct egalax_data *td, struct input_dev *input)
+{
+	struct egalax_contact *s = &td->single;
+	struct egalax_contact *best = 0;
+	int dbest, i;
+
+	for (i = 0; i < MAX_SLOTS; i++) {
+		struct egalax_contact *f = &td->contact[i];
+		if (f->touch) {
+			int d = abs(f->x - s->x) + abs(f->y - s->y);
+			if (!best || d < dbest) {
+				best = f;
+				dbest = d;
+			}
+		}
+	}
+
+	if (best) {
+		input_event(input, EV_KEY, BTN_TOUCH, 1);
+		input_event(input, EV_ABS, ABS_X, best->x);
+		input_event(input, EV_ABS, ABS_Y, best->y);
+		input_event(input, EV_ABS, ABS_PRESSURE, best->z);
+		*s = *best;
+	} else {
+		input_event(input, EV_KEY, BTN_TOUCH, 0);
+	}
+}
+
+
 /*
  * this function is called when a whole finger has been parsed,
  * so that it can decide what to send to the input layer.
  */
 static void egalax_filter_event(struct egalax_data *td, struct input_dev *input)
 {
-	td->first = !td->first; /* touchscreen emulation */
-
-	if (td->valid) {
-		/* emit multitouch events */
-		input_event(input, EV_ABS, ABS_MT_TRACKING_ID, td->id);
-		input_event(input, EV_ABS, ABS_MT_POSITION_X, td->x >> 3);
-		input_event(input, EV_ABS, ABS_MT_POSITION_Y, td->y >> 3);
-		input_event(input, EV_ABS, ABS_MT_PRESSURE, td->z);
-
-		input_mt_sync(input);
-
-		/*
-		 * touchscreen emulation: store (x, y) as
-		 * the last valid values in this frame
-		 */
-		td->lastx = td->x;
-		td->lasty = td->y;
-		td->lastz = td->z;
-	}
-
-	/*
-	 * touchscreen emulation: if this is the second finger and at least
-	 * one in this frame is valid, the latest valid in the frame is
-	 * the oldest on the panel, the one we want for single touch
-	 */
-	if (!td->first && td->activity) {
-		input_event(input, EV_ABS, ABS_X, td->lastx >> 3);
-		input_event(input, EV_ABS, ABS_Y, td->lasty >> 3);
- 		input_event(input, EV_ABS, ABS_PRESSURE, td->lastz);
-	}
-
-	if (!td->valid) {
-		/*
-		 * touchscreen emulation: if the first finger is invalid
-		 * and there previously was finger activity, this is a release
-		 */ 
-		if (td->first && td->activity) {
-			input_event(input, EV_KEY, BTN_TOUCH, 0);
-			td->activity = false;
+	struct egalax_contact *old = &td->contact[td->slot];
+	struct egalax_contact *f = &td->tmp;
+
+	input_mt_slot(input, td->slot);
+	if (f->touch) {
+		if (!old->touch) {
+			int id = td->trkid++ & MAX_TRKID;
+			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, id);
 		}
-		return;
+		input_event(input, EV_ABS, ABS_MT_POSITION_X, f->x);
+		input_event(input, EV_ABS, ABS_MT_POSITION_Y, f->y);
+		input_event(input, EV_ABS, ABS_MT_PRESSURE, f->z);
+	} else {
+		input_event(input, EV_ABS, ABS_MT_TRACKING_ID, -1);
 	}
+	*old = *f;
 
-
-	/* touchscreen emulation: if no previous activity, emit touch event */
-	if (!td->activity) {
-		input_event(input, EV_KEY, BTN_TOUCH, 1);
-		td->activity = true;
-	}
+	emulate_pointer(td, input);
 }
 
-
 static int egalax_event(struct hid_device *hid, struct hid_field *field,
 				struct hid_usage *usage, __s32 value)
 {
@@ -189,37 +197,38 @@ static int egalax_event(struct hid_device *hid, struct hid_field *field,
 	 * uses a standard parallel multitouch protocol (product ID ==
 	 * 48xx).  The second is capacitive and uses an unusual "serial"
 	 * protocol with a different message for each multitouch finger
-	 * (product ID == 72xx).  We do not yet generate a correct event
-	 * sequence for the capacitive/serial protocol.
+	 * (product ID == 72xx).
 	 */
 	if (hid->claimed & HID_CLAIMED_INPUT) {
 		struct input_dev *input = field->hidinput->input;
 
 		switch (usage->hid) {
 		case HID_DG_INRANGE:
+			td->valid = value;
+			break;
 		case HID_DG_CONFIDENCE:
 			/* avoid interference from generic hidinput handling */
 			break;
 		case HID_DG_TIPSWITCH:
-			td->valid = value;
+			td->tmp.touch = value;
 			break;
 		case HID_DG_TIPPRESSURE:
-			td->z = value;
+			td->tmp.z = value;
 			break;
 		case HID_DG_CONTACTID:
-			td->id = value;
+			td->slot = clamp_val(value, 0, MAX_SLOTS - 1);
 			break;
 		case HID_GD_X:
-			td->x = value;
+			td->tmp.x = value;
 			break;
 		case HID_GD_Y:
-			td->y = value;
+			td->tmp.y = value;
 			/* this is the last field in a finger */
-			egalax_filter_event(td, input);
+			if (td->valid)
+				egalax_filter_event(td, input);
 			break;
 		case HID_DG_CONTACTCOUNT:
 			/* touch emulation: this is the last field in a frame */
-			td->first = false;
 			break;
 
 		default:
-- 
1.7.1

Re: [PATCH 6/6] hid: egalax: Convert to MT slots

[Chase Douglas]

On Wed, 2010-10-13 at 15:58 +0200, Henrik Rydberg wrote:
> The joojoo reports touches sequentially, one per report, which
> confuses the current driver. Convert to the MT slots protocol and use
> the stored slot information to emulate pointer movement in a stable
> manner.
> 
> Tested-by: Philipp Merkel <mail@philmerk.de>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>

[...]

> @@ -122,64 +133,61 @@ static int egalax_input_mapped(struct hid_device *hdev, struct hid_input *hi,
>  	return -1;
>  }
>  
> +static void emulate_pointer(struct egalax_data *td, struct input_dev *input)
> +{
> +	struct egalax_contact *s = &td->single;
> +	struct egalax_contact *best = 0;
> +	int dbest, i;
> +
> +	for (i = 0; i < MAX_SLOTS; i++) {
> +		struct egalax_contact *f = &td->contact[i];
> +		if (f->touch) {
> +			int d = abs(f->x - s->x) + abs(f->y - s->y);
> +			if (!best || d < dbest) {
> +				best = f;
> +				dbest = d;
> +			}

Wouldn't it be better to do a real distance comparison:

int d = (f->x - s->x) * (f->x - s->x) + (f->y - s->y) * (f->y - s->y)

That should be the square of the actual distance between two contacts.

The rest looks ok to me.

-- Chase

Re: [PATCH 1/6] hid: egalax: Use kzalloc

[Chase Douglas]

On Wed, 2010-10-13 at 15:58 +0200, Henrik Rydberg wrote:
> To avoid unnecessary explicit initialization, allocate zeroed memory.
> 
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
>  drivers/hid/hid-egalax.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
> index 54b017a..5a1b52e 100644
> --- a/drivers/hid/hid-egalax.c
> +++ b/drivers/hid/hid-egalax.c
> @@ -221,7 +221,7 @@ static int egalax_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	struct egalax_data *td;
>  	struct hid_report *report;
>  
> -	td = kmalloc(sizeof(struct egalax_data), GFP_KERNEL);
> +	td = kzalloc(sizeof(struct egalax_data), GFP_KERNEL);
>  	if (!td) {
>  		dev_err(&hdev->dev, "cannot allocate eGalax data\n");
>  		return -ENOMEM;

Pretty obvious.

Acked-by: Chase Douglas <chase.douglas@canonical.com>

Re: [PATCH 2/6] hid: egalax: Setup input device manually

[Chase Douglas]

On Wed, 2010-10-13 at 15:58 +0200, Henrik Rydberg wrote:
> The hid core does not yet handle input filtering. Take over the setup
> of the input device, so that proper signal-to-noise ratios can be
> used.
> 
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
>  drivers/hid/hid-egalax.c |   37 +++++++++++++++++++++++++------------
>  1 files changed, 25 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
> index 5a1b52e..23f832b 100644
> --- a/drivers/hid/hid-egalax.c
> +++ b/drivers/hid/hid-egalax.c
> @@ -25,6 +25,8 @@ MODULE_LICENSE("GPL");
>  
>  #include "hid-ids.h"
>  
> +#define MAX_EVENTS		120
> +
>  struct egalax_data {
>  	__u16 x, y, z;
>  	__u8 id;
> @@ -34,10 +36,21 @@ struct egalax_data {
>  	__u16 lastx, lasty, lastz;	/* latest valid (x, y, z) in the frame */
>  };
>  
> +static void set_abs(struct input_dev *input, unsigned int code,
> +		    struct hid_field *field, int snratio)
> +{
> +	int fmin = field->logical_minimum;
> +	int fmax = field->logical_maximum;
> +	int fuzz = snratio ? (fmax - fmin) / snratio : 0;
> +	input_set_abs_params(input, code, fmin, fmax, fuzz, 0);
> +}
> +
>  static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  		struct hid_field *field, struct hid_usage *usage,
>  		unsigned long **bit, int *max)
>  {
> +	struct input_dev *input = hi->input;
> +
>  	switch (usage->hid & HID_USAGE_PAGE) {
>  
>  	case HID_UP_GENDESK:
> @@ -45,18 +58,16 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  		case HID_GD_X:
>  			hid_map_usage(hi, usage, bit, max,
>  					EV_ABS, ABS_MT_POSITION_X);
> +			set_abs(input, ABS_MT_POSITION_X, field, 0);
>  			/* touchscreen emulation */
> -			input_set_abs_params(hi->input, ABS_X,
> -						field->logical_minimum,
> -						field->logical_maximum, 0, 0);
> +			set_abs(input, ABS_X, field, 0);
>  			return 1;
>  		case HID_GD_Y:
>  			hid_map_usage(hi, usage, bit, max,
>  					EV_ABS, ABS_MT_POSITION_Y);
> +			set_abs(input, ABS_MT_POSITION_Y, field, 0);
>  			/* touchscreen emulation */
> -			input_set_abs_params(hi->input, ABS_Y,
> -						field->logical_minimum,
> -						field->logical_maximum, 0, 0);
> +			set_abs(input, ABS_Y, field, 0);
>  			return 1;
>  		}
>  		return 0;
> @@ -66,6 +77,7 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  		case HID_DG_TIPSWITCH:
>  			/* touchscreen emulation */
>  			hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
> +			input_set_capability(input, EV_KEY, BTN_TOUCH);
>  			return 1;
>  		case HID_DG_INRANGE:
>  		case HID_DG_CONFIDENCE:
> @@ -75,14 +87,15 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  		case HID_DG_CONTACTID:
>  			hid_map_usage(hi, usage, bit, max,
>  					EV_ABS, ABS_MT_TRACKING_ID);
> +			set_abs(input, ABS_MT_TRACKING_ID, field, 0);
> +			input_set_events_per_packet(input, MAX_EVENTS);
>  			return 1;
>  		case HID_DG_TIPPRESSURE:
>  			hid_map_usage(hi, usage, bit, max,
>  					EV_ABS, ABS_MT_PRESSURE);
> +			set_abs(input, ABS_MT_PRESSURE, field, 0);
>  			/* touchscreen emulation */
> -			input_set_abs_params(hi->input, ABS_PRESSURE,
> -						field->logical_minimum,
> -						field->logical_maximum, 0, 0);
> +			set_abs(input, ABS_PRESSURE, field, 0);
>  			return 1;
>  		}
>  		return 0;
> @@ -96,10 +109,10 @@ static int egalax_input_mapped(struct hid_device *hdev, struct hid_input *hi,
>  		struct hid_field *field, struct hid_usage *usage,
>  		unsigned long **bit, int *max)
>  {
> +	/* tell hid-input to skip setup of these event types */
>  	if (usage->type == EV_KEY || usage->type == EV_ABS)
> -		clear_bit(usage->code, *bit);
> -
> -	return 0;
> +		set_bit(usage->type, hi->input->evbit);
> +	return -1;
>  }
>  
>  /*

Seems reasonable to me. For others, it's not obvious what this changes
until you see patch 5/6. I'm curious why you don't just squash the two
patches together?

Acked-by: Chase Douglas <chase.douglas@canonical.com>

Re: [PATCH 3/6] hid: egalax: Correct for device resolution report error

[Chase Douglas]

On Wed, 2010-10-13 at 15:58 +0200, Henrik Rydberg wrote:
> The firmware of both supported devices report a X/Y maximum of 4096,
> whereas in reality, it is eight times larger. Fixed with this patch.
> 
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
>  drivers/hid/hid-egalax.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
> index 23f832b..3b490a3 100644
> --- a/drivers/hid/hid-egalax.c
> +++ b/drivers/hid/hid-egalax.c
> @@ -56,6 +56,7 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  	case HID_UP_GENDESK:
>  		switch (usage->hid) {
>  		case HID_GD_X:
> +			field->logical_maximum <<= 3;
>  			hid_map_usage(hi, usage, bit, max,
>  					EV_ABS, ABS_MT_POSITION_X);
>  			set_abs(input, ABS_MT_POSITION_X, field, 0);
> @@ -63,6 +64,7 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  			set_abs(input, ABS_X, field, 0);
>  			return 1;
>  		case HID_GD_Y:
> +			field->logical_maximum <<= 3;
>  			hid_map_usage(hi, usage, bit, max,
>  					EV_ABS, ABS_MT_POSITION_Y);
>  			set_abs(input, ABS_MT_POSITION_Y, field, 0);

Seems obvious.

Acked-by: Chase Douglas <chase.douglas@canonical.com>

Re: [PATCH 4/6] hid: egalax: Report zero as minimum pressure

[Chase Douglas]

On Wed, 2010-10-13 at 15:58 +0200, Henrik Rydberg wrote:
> The firmware reports a logical minimum of one, but in order for
> userspace applications to correctly map all reported values to
> non-zero pressure, the driver needs to report a logical minimum of
> zero.  Fixed with this patch.
> 
> Tested-by: Philipp Merkel <mail@philmerk.de>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
>  drivers/hid/hid-egalax.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
> index 3b490a3..2aba897 100644
> --- a/drivers/hid/hid-egalax.c
> +++ b/drivers/hid/hid-egalax.c
> @@ -93,6 +93,7 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  			input_set_events_per_packet(input, MAX_EVENTS);
>  			return 1;
>  		case HID_DG_TIPPRESSURE:
> +			field->logical_minimum = 0;
>  			hid_map_usage(hi, usage, bit, max,
>  					EV_ABS, ABS_MT_PRESSURE);
>  			set_abs(input, ABS_MT_PRESSURE, field, 0);

I'm a little unsure of whether the logical_minimum field should be
modified by the driver or if the range of the evdev property should be
the only change. I can't see anything in the hid code that looks like it
would be adversely affected by this change though.

-- Chase

Re: [PATCH 5/6] hid: egalax: Add event filtering

[Chase Douglas]

On Wed, 2010-10-13 at 15:58 +0200, Henrik Rydberg wrote:
> Use estimated signal-to-noise ratios to reduce noise and limit the
> amount of events emitted.
> 
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
>  drivers/hid/hid-egalax.c |   16 ++++++++++------
>  1 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
> index 2aba897..b21e7dc 100644
> --- a/drivers/hid/hid-egalax.c
> +++ b/drivers/hid/hid-egalax.c
> @@ -27,6 +27,10 @@ MODULE_LICENSE("GPL");
>  
>  #define MAX_EVENTS		120
>  
> +/* estimated signal-to-noise ratios */
> +#define SN_MOVE			1024
> +#define SN_PRESSURE		32
> +
>  struct egalax_data {
>  	__u16 x, y, z;
>  	__u8 id;
> @@ -59,17 +63,17 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  			field->logical_maximum <<= 3;
>  			hid_map_usage(hi, usage, bit, max,
>  					EV_ABS, ABS_MT_POSITION_X);
> -			set_abs(input, ABS_MT_POSITION_X, field, 0);
> +			set_abs(input, ABS_MT_POSITION_X, field, SN_MOVE);
>  			/* touchscreen emulation */
> -			set_abs(input, ABS_X, field, 0);
> +			set_abs(input, ABS_X, field, SN_MOVE);
>  			return 1;
>  		case HID_GD_Y:
>  			field->logical_maximum <<= 3;
>  			hid_map_usage(hi, usage, bit, max,
>  					EV_ABS, ABS_MT_POSITION_Y);
> -			set_abs(input, ABS_MT_POSITION_Y, field, 0);
> +			set_abs(input, ABS_MT_POSITION_Y, field, SN_MOVE);
>  			/* touchscreen emulation */
> -			set_abs(input, ABS_Y, field, 0);
> +			set_abs(input, ABS_Y, field, SN_MOVE);
>  			return 1;
>  		}
>  		return 0;
> @@ -96,9 +100,9 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  			field->logical_minimum = 0;
>  			hid_map_usage(hi, usage, bit, max,
>  					EV_ABS, ABS_MT_PRESSURE);
> -			set_abs(input, ABS_MT_PRESSURE, field, 0);
> +			set_abs(input, ABS_MT_PRESSURE, field, SN_PRESSURE);
>  			/* touchscreen emulation */
> -			set_abs(input, ABS_PRESSURE, field, 0);
> +			set_abs(input, ABS_PRESSURE, field, SN_PRESSURE);
>  			return 1;
>  		}
>  		return 0;

Seems obvious.

Acked-by: Chase Douglas <chase.douglas@canonical.com>

Re: [PATCH 3/6] hid: egalax: Correct for device resolution report error

[Henrik Rydberg]

On 10/13/2010 03:58 PM, Henrik Rydberg wrote:

> The firmware of both supported devices report a X/Y maximum of 4096,
> whereas in reality, it is eight times larger. Fixed with this patch.
> 
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
>  drivers/hid/hid-egalax.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
> index 23f832b..3b490a3 100644
> --- a/drivers/hid/hid-egalax.c
> +++ b/drivers/hid/hid-egalax.c
> @@ -56,6 +56,7 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  	case HID_UP_GENDESK:
>  		switch (usage->hid) {
>  		case HID_GD_X:
> +			field->logical_maximum <<= 3;


This makes the logical maximum increase by a factor of eight every time the
module is loaded.

>  			hid_map_usage(hi, usage, bit, max,
>  					EV_ABS, ABS_MT_POSITION_X);
>  			set_abs(input, ABS_MT_POSITION_X, field, 0);
> @@ -63,6 +64,7 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  			set_abs(input, ABS_X, field, 0);
>  			return 1;
>  		case HID_GD_Y:
> +			field->logical_maximum <<= 3;


Same here.

>  			hid_map_usage(hi, usage, bit, max,
>  					EV_ABS, ABS_MT_POSITION_Y);
>  			set_abs(input, ABS_MT_POSITION_Y, field, 0);


Will send new revisions of patches 3 and 5...

Henrik

[PATCH 3/6] hid: egalax: Correct for device resolution report error (rev2)

[Henrik Rydberg]

The firmware of both supported devices report a X/Y maximum of 4095,
whereas in reality, it is eight times larger. Fixed with this patch.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/hid/hid-egalax.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
index 23f832b..6c446b0 100644
--- a/drivers/hid/hid-egalax.c
+++ b/drivers/hid/hid-egalax.c
@@ -56,6 +56,7 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	case HID_UP_GENDESK:
 		switch (usage->hid) {
 		case HID_GD_X:
+			field->logical_maximum = 32760;
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_POSITION_X);
 			set_abs(input, ABS_MT_POSITION_X, field, 0);
@@ -63,6 +64,7 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			set_abs(input, ABS_X, field, 0);
 			return 1;
 		case HID_GD_Y:
+			field->logical_maximum = 32760;
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_POSITION_Y);
 			set_abs(input, ABS_MT_POSITION_Y, field, 0);
-- 
1.7.1

[PATCH 5/6] hid: egalax: Add event filtering (rev2)

[Henrik Rydberg]

Use estimated signal-to-noise ratios to reduce noise and limit the
amount of events emitted.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/hid/hid-egalax.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
index b8025b9..78a2b3e 100644
--- a/drivers/hid/hid-egalax.c
+++ b/drivers/hid/hid-egalax.c
@@ -27,6 +27,10 @@ MODULE_LICENSE("GPL");
 
 #define MAX_EVENTS		120
 
+/* estimated signal-to-noise ratios */
+#define SN_MOVE			1024
+#define SN_PRESSURE		32
+
 struct egalax_data {
 	__u16 x, y, z;
 	__u8 id;
@@ -59,17 +63,17 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			field->logical_maximum = 32760;
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_POSITION_X);
-			set_abs(input, ABS_MT_POSITION_X, field, 0);
+			set_abs(input, ABS_MT_POSITION_X, field, SN_MOVE);
 			/* touchscreen emulation */
-			set_abs(input, ABS_X, field, 0);
+			set_abs(input, ABS_X, field, SN_MOVE);
 			return 1;
 		case HID_GD_Y:
 			field->logical_maximum = 32760;
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_POSITION_Y);
-			set_abs(input, ABS_MT_POSITION_Y, field, 0);
+			set_abs(input, ABS_MT_POSITION_Y, field, SN_MOVE);
 			/* touchscreen emulation */
-			set_abs(input, ABS_Y, field, 0);
+			set_abs(input, ABS_Y, field, SN_MOVE);
 			return 1;
 		}
 		return 0;
@@ -96,9 +100,9 @@ static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			field->logical_minimum = 0;
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_PRESSURE);
-			set_abs(input, ABS_MT_PRESSURE, field, 0);
+			set_abs(input, ABS_MT_PRESSURE, field, SN_PRESSURE);
 			/* touchscreen emulation */
-			set_abs(input, ABS_PRESSURE, field, 0);
+			set_abs(input, ABS_PRESSURE, field, SN_PRESSURE);
 			return 1;
 		}
 		return 0;
-- 
1.7.1

Re: [PATCH 0/6] hid: egalax: Rework to include joojoo support

[Jiri Kosina]

On Wed, 13 Oct 2010, Henrik Rydberg wrote:

> Hi Jiri, Stephane,
> 
> This series adds support for the joojoo tablet. Based on the
> jikos/egalax branch, the first two patches prepare for the change to
> MT slots. The third patch fixes the bug that was also fixed by commit
> f516611, but in another (agreed) way. The fourth patch fixes another
> minor problem. The fifth patch adds event filtering based on observed
> signal quality, and the sixth patch does the heavy lifting of
> converting to MT slots and thereby solving the major problem for the
> joojoo.
> 
> The patches have been tested on both supported devices. Thanks Philipp!
> 
> Henrik
> 
> Henrik Rydberg (6):
>   hid: egalax: Use kzalloc
>   hid: egalax: Setup input device manually
>   hid: egalax: Correct for device resolution report error
>   hid: egalax: Report zero as minimum pressure
>   hid: egalax: Add event filtering
>   hid: egalax: Convert to MT slots
> 
>  drivers/hid/hid-egalax.c |  177 +++++++++++++++++++++++++++-------------------
>  1 files changed, 103 insertions(+), 74 deletions(-)

Stephane, I'd appreciate if I could get your Acked-by: for the series. 

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 1/6] hid: egalax: Use kzalloc

[Jiri Kosina]

On Wed, 13 Oct 2010, Chase Douglas wrote:

> > To avoid unnecessary explicit initialization, allocate zeroed memory.
> > 
> > Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> > ---
> >  drivers/hid/hid-egalax.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
> > index 54b017a..5a1b52e 100644
> > --- a/drivers/hid/hid-egalax.c
> > +++ b/drivers/hid/hid-egalax.c
> > @@ -221,7 +221,7 @@ static int egalax_probe(struct hid_device *hdev, const struct hid_device_id *id)
> >  	struct egalax_data *td;
> >  	struct hid_report *report;
> >  
> > -	td = kmalloc(sizeof(struct egalax_data), GFP_KERNEL);
> > +	td = kzalloc(sizeof(struct egalax_data), GFP_KERNEL);
> >  	if (!td) {
> >  		dev_err(&hdev->dev, "cannot allocate eGalax data\n");
> >  		return -ENOMEM;
> 
> Pretty obvious.
> 
> Acked-by: Chase Douglas <chase.douglas@canonical.com>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 0/6] hid: egalax: Rework to include joojoo support

[Jiri Kosina]

On Wed, 13 Oct 2010, Henrik Rydberg wrote:

> Hi Jiri, Stephane,
> 
> This series adds support for the joojoo tablet. Based on the
> jikos/egalax branch, the first two patches prepare for the change to
> MT slots. The third patch fixes the bug that was also fixed by commit
> f516611, but in another (agreed) way. The fourth patch fixes another
> minor problem. The fifth patch adds event filtering based on observed
> signal quality, and the sixth patch does the heavy lifting of
> converting to MT slots and thereby solving the major problem for the
> joojoo.
> 
> The patches have been tested on both supported devices. Thanks Philipp!
> 
> Henrik
> 
> Henrik Rydberg (6):
>   hid: egalax: Use kzalloc
>   hid: egalax: Setup input device manually
>   hid: egalax: Correct for device resolution report error
>   hid: egalax: Report zero as minimum pressure
>   hid: egalax: Add event filtering
>   hid: egalax: Convert to MT slots
> 
>  drivers/hid/hid-egalax.c |  177 +++++++++++++++++++++++++++-------------------
>  1 files changed, 103 insertions(+), 74 deletions(-)

I have applied the really really obvious one. Stephane, could you please 
send me your Acked-by: for the rest? Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.