[PATCH 1/2 v2] HID: magicmouse: Removing report_touches switch

Subsystems: hid core layer, the rest · Maintainers: Jiri Kosina, Benjamin Tissoires, Linus Torvalds

STALE5227d

6 messages, 4 authors, 2012-07-04 · open the first message on its own page

[PATCH 1/2 v2] HID: magicmouse: Removing report_touches switch

From: Yufeng Shen <hidden>
Date: 2012-07-03 18:53:51

Remove the report_touches switch as it is not so useful to turn
off reporting touch events for a touch device. Let the userspace
to do the filtering if the turning off is needed.

V2: Remove report_touches as suggeted by Chase Douglas

Signed-off-by: Yufeng Shen <redacted>
---
 drivers/hid/hid-magicmouse.c |   95 ++++++++++++++++++++----------------------
 1 files changed, 45 insertions(+), 50 deletions(-)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 40ac665..fd88f21 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c

@@ -48,10 +48,6 @@ static bool scroll_acceleration = false;
 module_param(scroll_acceleration, bool, 0644);
 MODULE_PARM_DESC(scroll_acceleration, "Accelerate sequential scroll events");
 
-static bool report_touches = true;
-module_param(report_touches, bool, 0644);
-MODULE_PARM_DESC(report_touches, "Emit touch records (otherwise, only use them for emulation)");
-
 static bool report_undeciphered;
 module_param(report_undeciphered, bool, 0644);
 MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state field using a MSC_RAW event");

@@ -276,7 +272,7 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
 		msc->single_touch_id = SINGLE_TOUCH_UP;
 
 	/* Generate the input events for this touch. */
-	if (report_touches && down) {
+	if (down) {
 		input_report_abs(input, ABS_MT_TRACKING_ID, id);
 		input_report_abs(input, ABS_MT_TOUCH_MAJOR, touch_major << 2);
 		input_report_abs(input, ABS_MT_TOUCH_MINOR, touch_minor << 2);

@@ -335,7 +331,7 @@ static int magicmouse_raw_event(struct hid_device *hdev,
 		for (ii = 0; ii < npoints; ii++)
 			magicmouse_emit_touch(msc, ii, data + ii * 8 + 6);
 
-		if (report_touches && msc->ntouches == 0)
+		if (msc->ntouches == 0)
 			input_mt_sync(input);
 
 		/* When emulating three-button mode, it is important

@@ -422,53 +418,52 @@ static void magicmouse_setup_input(struct input_dev *input, struct hid_device *h
 		__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
 	}
 
-	if (report_touches) {
-		__set_bit(EV_ABS, input->evbit);
-
-		input_set_abs_params(input, ABS_MT_TRACKING_ID, 0, 15, 0, 0);
-		input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 255 << 2,
-				     4, 0);
-		input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0, 255 << 2,
-				     4, 0);
-		input_set_abs_params(input, ABS_MT_ORIENTATION, -31, 32, 1, 0);
-
-		/* Note: Touch Y position from the device is inverted relative
-		 * to how pointer motion is reported (and relative to how USB
-		 * HID recommends the coordinates work).  This driver keeps
-		 * the origin at the same position, and just uses the additive
-		 * inverse of the reported Y.
-		 */
-		if (input->id.product == USB_DEVICE_ID_APPLE_MAGICMOUSE) {
-			input_set_abs_params(input, ABS_MT_POSITION_X,
-				MOUSE_MIN_X, MOUSE_MAX_X, 4, 0);
-			input_set_abs_params(input, ABS_MT_POSITION_Y,
-				MOUSE_MIN_Y, MOUSE_MAX_Y, 4, 0);
-
-			input_abs_set_res(input, ABS_MT_POSITION_X,
-				MOUSE_RES_X);
-			input_abs_set_res(input, ABS_MT_POSITION_Y,
-				MOUSE_RES_Y);
-		} else { /* USB_DEVICE_ID_APPLE_MAGICTRACKPAD */
-			input_set_abs_params(input, ABS_X, TRACKPAD_MIN_X,
-				TRACKPAD_MAX_X, 4, 0);
-			input_set_abs_params(input, ABS_Y, TRACKPAD_MIN_Y,
-				TRACKPAD_MAX_Y, 4, 0);
-			input_set_abs_params(input, ABS_MT_POSITION_X,
-				TRACKPAD_MIN_X, TRACKPAD_MAX_X, 4, 0);
-			input_set_abs_params(input, ABS_MT_POSITION_Y,
-				TRACKPAD_MIN_Y, TRACKPAD_MAX_Y, 4, 0);
-
-			input_abs_set_res(input, ABS_X, TRACKPAD_RES_X);
-			input_abs_set_res(input, ABS_Y, TRACKPAD_RES_Y);
-			input_abs_set_res(input, ABS_MT_POSITION_X,
-				TRACKPAD_RES_X);
-			input_abs_set_res(input, ABS_MT_POSITION_Y,
-				TRACKPAD_RES_Y);
-		}
 
-		input_set_events_per_packet(input, 60);
+	__set_bit(EV_ABS, input->evbit);
+
+	input_set_abs_params(input, ABS_MT_TRACKING_ID, 0, 15, 0, 0);
+	input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 255 << 2,
+			     4, 0);
+	input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0, 255 << 2,
+			     4, 0);
+	input_set_abs_params(input, ABS_MT_ORIENTATION, -31, 32, 1, 0);
+
+	/* Note: Touch Y position from the device is inverted relative
+	 * to how pointer motion is reported (and relative to how USB
+	 * HID recommends the coordinates work).  This driver keeps
+	 * the origin at the same position, and just uses the additive
+	 * inverse of the reported Y.
+	 */
+	if (input->id.product == USB_DEVICE_ID_APPLE_MAGICMOUSE) {
+		input_set_abs_params(input, ABS_MT_POSITION_X,
+				     MOUSE_MIN_X, MOUSE_MAX_X, 4, 0);
+		input_set_abs_params(input, ABS_MT_POSITION_Y,
+				     MOUSE_MIN_Y, MOUSE_MAX_Y, 4, 0);
+
+		input_abs_set_res(input, ABS_MT_POSITION_X,
+				  MOUSE_RES_X);
+		input_abs_set_res(input, ABS_MT_POSITION_Y,
+				  MOUSE_RES_Y);
+	} else { /* USB_DEVICE_ID_APPLE_MAGICTRACKPAD */
+		input_set_abs_params(input, ABS_X, TRACKPAD_MIN_X,
+				     TRACKPAD_MAX_X, 4, 0);
+		input_set_abs_params(input, ABS_Y, TRACKPAD_MIN_Y,
+				     TRACKPAD_MAX_Y, 4, 0);
+		input_set_abs_params(input, ABS_MT_POSITION_X,
+				     TRACKPAD_MIN_X, TRACKPAD_MAX_X, 4, 0);
+		input_set_abs_params(input, ABS_MT_POSITION_Y,
+				     TRACKPAD_MIN_Y, TRACKPAD_MAX_Y, 4, 0);
+
+		input_abs_set_res(input, ABS_X, TRACKPAD_RES_X);
+		input_abs_set_res(input, ABS_Y, TRACKPAD_RES_Y);
+		input_abs_set_res(input, ABS_MT_POSITION_X,
+				  TRACKPAD_RES_X);
+		input_abs_set_res(input, ABS_MT_POSITION_Y,
+				  TRACKPAD_RES_Y);
 	}
 
+	input_set_events_per_packet(input, 60);
+
 	if (report_undeciphered) {
 		__set_bit(EV_MSC, input->evbit);
 		__set_bit(MSC_RAW, input->mscbit);

-- 
1.7.7.3

Re: [PATCH 1/2 v2] HID: magicmouse: Removing report_touches switch

From: Chase Douglas <hidden>
Date: 2012-07-03 20:27:09

On 07/03/2012 11:53 AM, Yufeng Shen wrote:

Remove the report_touches switch as it is not so useful to turn
off reporting touch events for a touch device. Let the userspace
to do the filtering if the turning off is needed.

V2: Remove report_touches as suggeted by Chase Douglas

Signed-off-by: Yufeng Shen <redacted>

Acked-by: Chase Douglas <redacted>

Thanks!

quoted hunk

---
  drivers/hid/hid-magicmouse.c |   95 ++++++++++++++++++++----------------------
  1 files changed, 45 insertions(+), 50 deletions(-)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 40ac665..fd88f21 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c

@@ -48,10 +48,6 @@ static bool scroll_acceleration = false;
  module_param(scroll_acceleration, bool, 0644);
  MODULE_PARM_DESC(scroll_acceleration, "Accelerate sequential scroll events");

-static bool report_touches = true;
-module_param(report_touches, bool, 0644);
-MODULE_PARM_DESC(report_touches, "Emit touch records (otherwise, only use them for emulation)");
-
  static bool report_undeciphered;
  module_param(report_undeciphered, bool, 0644);
  MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state field using a MSC_RAW event");

@@ -276,7 +272,7 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
  		msc->single_touch_id = SINGLE_TOUCH_UP;

  	/* Generate the input events for this touch. */
-	if (report_touches && down) {
+	if (down) {
  		input_report_abs(input, ABS_MT_TRACKING_ID, id);
  		input_report_abs(input, ABS_MT_TOUCH_MAJOR, touch_major << 2);
  		input_report_abs(input, ABS_MT_TOUCH_MINOR, touch_minor << 2);

@@ -335,7 +331,7 @@ static int magicmouse_raw_event(struct hid_device *hdev,
  		for (ii = 0; ii < npoints; ii++)
  			magicmouse_emit_touch(msc, ii, data + ii * 8 + 6);

-		if (report_touches && msc->ntouches == 0)
+		if (msc->ntouches == 0)
  			input_mt_sync(input);

  		/* When emulating three-button mode, it is important

@@ -422,53 +418,52 @@ static void magicmouse_setup_input(struct input_dev *input, struct hid_device *h
  		__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
  	}

95 more quoted lines

Re: [PATCH 1/2 v2] HID: magicmouse: Removing report_touches switch

From: Henrik Rydberg <hidden>
Date: 2012-07-04 07:40:31

On Tue, Jul 03, 2012 at 01:27:03PM -0700, Chase Douglas wrote:

On 07/03/2012 11:53 AM, Yufeng Shen wrote:

quoted

Remove the report_touches switch as it is not so useful to turn
off reporting touch events for a touch device. Let the userspace
to do the filtering if the turning off is needed.

V2: Remove report_touches as suggeted by Chase Douglas

Signed-off-by: Yufeng Shen <redacted>

Acked-by: Chase Douglas <redacted>

Hi Chase,

While testing the new patches, I noticed that only four fingers would
come through; a fifth finger would make everything freeze. This
behavior is also present without the new patches. Didn't we have ten
fingers working at some point?

Cheers,
Henrik

Re: [PATCH 1/2 v2] HID: magicmouse: Removing report_touches switch

From: Daniel Kurtz <hidden>
Date: 2012-07-04 07:50:13

On Wed, Jul 4, 2012 at 3:40 PM, Henrik Rydberg [off-list ref] wrote:

On Tue, Jul 03, 2012 at 01:27:03PM -0700, Chase Douglas wrote:

quoted

On 07/03/2012 11:53 AM, Yufeng Shen wrote:

quoted

Remove the report_touches switch as it is not so useful to turn
off reporting touch events for a touch device. Let the userspace
to do the filtering if the turning off is needed.

V2: Remove report_touches as suggeted by Chase Douglas

Signed-off-by: Yufeng Shen <redacted>

Acked-by: Chase Douglas <redacted>

Hi Chase,

While testing the new patches, I noticed that only four fingers would
come through; a fifth finger would make everything freeze. This
behavior is also present without the new patches. Didn't we have ten
fingers working at some point?

Cheers,
Henrik

Hi Henrik,

What Bluetooth host controller are you using?
Which part are you testing (Magic TP or mouse)?

I've been trying to test these patches on a Magic Trackpad using two
computers, with two different ath3k parts.
On one of them, the BT communication freezes immediately after the
first report with 3 fingers.
On the other, it reports all 10 fingers, just fine.

The only difference I can tell is the version of the Atheros Bluetooth chip.

What does your freeze look like?

-Dan

Re: [PATCH 1/2 v2] HID: magicmouse: Removing report_touches switch

From: Henrik Rydberg <hidden>
Date: 2012-07-04 08:38:24

Hi Daniel,

What Bluetooth host controller are you using?

It is a USB device (0a5c:4500), part of a BCM2046 sitting in a MacBook
Air. The driver is btusb.

Which part are you testing (Magic TP or mouse)?

Magic trackpad.

I've been trying to test these patches on a Magic Trackpad using two
computers, with two different ath3k parts.
On one of them, the BT communication freezes immediately after the
first report with 3 fingers.
On the other, it reports all 10 fingers, just fine.

Interesting. Clearly not a HID problem, then.

The only difference I can tell is the version of the Atheros Bluetooth chip.

What does your freeze look like?

The input events stop coming when I have more than four fingers on the
pad, but they start again if I remove the excess fingers. As if the
transport layer chokes on messages longer than some value...

Thanks,
Henrik

Re: [PATCH 1/2 v2] HID: magicmouse: Removing report_touches switch

From: Henrik Rydberg <hidden>
Date: 2012-07-04 18:41:36

The input events stop coming when I have more than four fingers on the
pad, but they start again if I remove the excess fingers. As if the
transport layer chokes on messages longer than some value...

Yep, that's it. On my system, something sets the L2CAP MTU to a
whopping 48 bytes, and the code below, from l2cap_core.c,

	switch (chan->mode) {
	case L2CAP_MODE_BASIC:
		/* If socket recv buffers overflows we drop data here
		 * which is *bad* because L2CAP has to be reliable.
		 * But we don't have any other choice. L2CAP doesn't
		 * provide flow control mechanism. */

		if (chan->imtu < skb->len)
			goto drop;

silently ignores packets larger than that value. Duh.

Henrik

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help