Re: [PATCH] Input: Add support for Fujitsu S762 laptops scroll wheel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dimitry,

Thanks for your reply!

On 2013-05-21 20:58, Dmitry Torokhov wrote:
Hi Tamás,

On Tue, May 21, 2013 at 06:28:26PM +0200, Eisenberger Tamás wrote:
Hello,

This were my first contribution, and I received no response at all :(
Am I done something wrong, no one else is interested in supporting
this particular piece of hardware or what?

If anyone has a minute please review it.


Ah, yes, sorry...

Thanks:
Tamás

On 2013-05-06 19:35, Eisenberger Tamás wrote:
Detects and makes the Touch Scroll Wheel found on some Fujitsu laptops
working.

The detection is based on the (hopefully) unique E7 report of the wheel
device.
Up and down scrolling is detected by the only byte that has different
values in the devices packets, but it should be fine since it only able
to report that two events.

Signed-off-by: Tama's Eisenberger <tamas@xxxxxxxxxxxxxx>
Index: linux/drivers/input/mouse/alps.c
===================================================================
--- linux.orig/drivers/input/mouse/alps.c
+++ linux/drivers/input/mouse/alps.c
@@ -410,6 +410,17 @@ static void alps_process_trackstick_pack
  	if (packet[1] == 0x7f && packet[2] == 0x7f && packet[4] == 0x7f)
  		return;

+	if (priv->quirks & ALPS_QUIRK_SCROLL_WHEEL) {
+		if (packet[3] == 0x4c)
+			input_report_rel(dev, REL_WHEEL, 1);
+
+		if (packet[3] == 0x58)
+			input_report_rel(dev, REL_WHEEL, -1);

Are these really the only values that emitted by the wheel? Have you
tried spinning it vigorously to see if it produces different results?

I've tried to scroll really slow, and really fast with it, also tryed to move my finger diagonally and randomly but only that two were sent. Also events are really rare when not moving the finger around the edge.


+
+		input_sync(dev);
+		return;

Why are we ignoring the rest of the packet in this case? Does it contain
invalid data?

Without my modifications the original driver reports middle button events when I scroll upwards, so I have to return here to prevent this. I don't think that this scroll well can co-exists with a trackstick.


+	}
+
  	x = (s8)(((packet[0] & 0x20) << 2) | (packet[1] & 0x7f));
  	y = (s8)(((packet[0] & 0x10) << 3) | (packet[2] & 0x7f));
  	z = (packet[4] & 0x7c) >> 2;
@@ -1255,6 +1266,7 @@ error:
  static int alps_setup_trackstick_v3(struct psmouse *psmouse, int
reg_base)
  {
  	struct ps2dev *ps2dev = &psmouse->ps2dev;
+	struct alps_data *priv = psmouse->private;
  	int ret = 0;
  	unsigned char param[4];

@@ -1277,6 +1289,12 @@ static int alps_setup_trackstick_v3(stru
  		psmouse_dbg(psmouse, "trackstick E7 report: %3ph\n", param);

  		/*
+		 * Detect fujitsu scroll wheel device
+		 */
+		if (param[0] == 0x34 && param[1] == 0x01 && param[2] == 0x14)
+			priv->quirks |= ALPS_QUIRK_SCROLL_WHEEL;
+

I believe this quirk should go into alps_model_data[] table.

I've tried to do that but it seems that the exact same (as my primary touchpad) signature is already in the alps_model_data[] table (E7=73 02 64, EC=88 07 9d) and the secondary device seems to be independent. This is why I'm trying to use the EC report of the secondary device but it seemed to be overkill to create a new model_data array for attached devices or I don't know what could be the consequences to use the same array also for secondary devices.


+		/*
  		 * Not sure what this does, but it is absolutely
  		 * essential. Without it, the touchpad does not
  		 * work at all and the trackstick just emits normal
@@ -1798,12 +1816,16 @@ int alps_init(struct psmouse *psmouse)
  	dev2->id.product = PSMOUSE_ALPS;
  	dev2->id.version = 0x0000;
  	dev2->dev.parent = &psmouse->ps2dev.serio->dev;
-
  	dev2->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
  	dev2->relbit[BIT_WORD(REL_X)] = BIT_MASK(REL_X) | BIT_MASK(REL_Y);
  	dev2->keybit[BIT_WORD(BTN_LEFT)] =
  		BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) | BIT_MASK(BTN_RIGHT);

+	if (priv->quirks & ALPS_QUIRK_SCROLL_WHEEL) {
+		dev2->name = "DualPoint Scroll Wheel";

I'd keep the name as is.

No prob. I'l remove the name change.


+		dev2->relbit[BIT_WORD(REL_X)] |= BIT_MASK(REL_WHEEL);
+	}
+
  	if (input_register_device(priv->dev2))
  		goto init_fail;

Index: linux/drivers/input/mouse/alps.h
===================================================================
--- linux.orig/drivers/input/mouse/alps.h
+++ linux/drivers/input/mouse/alps.h
@@ -158,6 +158,7 @@ struct alps_data {
  };

  #define ALPS_QUIRK_TRACKSTICK_BUTTONS	1 /* trakcstick buttons in
trackstick packet */
+#define ALPS_QUIRK_SCROLL_WHEEL		2 /* secondary device is scroll wheel
*/

  #ifdef CONFIG_MOUSE_PS2_ALPS
  int alps_detect(struct psmouse *psmouse, bool set_properties);


BTW, your mailer line-wrapped the patch so it is not possible to apply,
when resending please make sure the long lines stay intact.

You're right, I’ll try to use an other one.


Thanks.


So basically as I see the main concern is the introduction of a new quirk, but I don't know how to do this using the alps_model_data[] if you have any suggestions please tell me!

Thanks:
Tamás
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux