[PATCH] Driver for Logitech M560

[PATCH] Driver for Logitech M560

[Goffredo Baroncelli]

Sorry,
I repost this email because I didn't see it in the mailing list
NR
G.Baroncelli

-------- Forwarded Message --------
Subject: Driver for Logitech M560
Date: Mon, 01 Sep 2014 21:20:20 +0200
From: Goffredo Baroncelli <kreijack@inwind.it>
Reply-To: kreijack@inwind.it
To: Benjamin Tissoires <benjamin.tissoires@gmail.com>
CC: Nestor Lopez Casado <nlopezcasad@logitech.com>, open list:HID CORE LAYER <linux-input@vger.kernel.org>

Hi Benjamin,

following the Nestor suggestion, I rewrote the driver for the 
mouse Logitech M560 on the basis of your work (branch "for-whot").

The Logitech M560 is is a mouse designed for windows 8. 
Comparing to a standard one, some buttons (the middle one and the 
two ones placed on the side) are bounded to a key combination 
instead of a classic "mouse" button.

Think this mouse as a pair of mouse and keyboard. When the middle
button is pressed the it sends a key (as keyboard) 
combination, the same for the other two side button.
Instead the left/right/wheel work correctly.
To complicate further the things, the middle button send a
key combination the odd press, and another one for the even press;
in the latter case it sends also a left click. But the worst thing 
is that no event is generated when the middle button is released.

Moreover this device is a wireless mouse which uses the unifying 
receiver.

I discovered that it is possible to re-configure the mouse
sending a command (see function m560_send_config_command()).
After this command the mouse sends some sequence when the
buttons are pressed and/or released (see comments for
an explanation of the mouse protocol).

If the mouse is "silent" (no data is sent) for more than 
PACKET_TIMEOUT seconds (currently 10), when the next packet
comes, the driver try to reconfigure it.
This because it is possible that the mouse is switched-off
and lost its setting. Se we need to reconfigure.

Benjamin, I have to highlight three things:
when the dj driver detects a disconnection, it
sends a sequence like

  0x01 0x00 0x00 0x00 0x00 0x00 0x00 ...
  0x02 0x00 0x00 0x00 0x00 0x00 0x00 ...

because the mouse is both a keyboard (0x01) and 
a mouse (0x02). But this sequence is a valid one
(when two buttons are released) due to the strangeness
of the protocol.

Can I suggest to send in these case a sequence like

  <device_type> 0xff 0xff 0xff 0xff 0xff 0xff....

? I suspect that this would be less common. 

Another thing that seemed strange to me is that report
with ID equal 0x20 and 0x21 are handled so the packet
are forwarded to the driver from the 3rd byte onward.
In the others cases (this mouse send also packet with 
ID=0x11) the report id forwarded as is (without
by-passing the first two bytes). It is the expected
behaviour, or the code was written on the basis the
assumption that the devices send ID=0x20/0x21 and the
other ID are sent only by the receiver ?

Finally I have to point out that to work, this driver
has to be inserted BEFORE the hid_logitech_dj

In fact I had to add a file /etc/modprobe.d/hid-logitech_dj.conf
as below

install hid_logitech_dj modprobe hid_logitech_m560; sleep 5s ; modprobe --ignore-install hid_logitech_dj

It is possible that the "sleep 5s" is not needed.

BR
G.Baroncelli

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5


---

This driver add support for the mouse Logitech m560

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>

diff --git a/Makefile b/Makefile
index 3016586..86157c0 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,7 @@
 obj-m	+= hid-logitech-dj.o
 obj-m	+= hid-logitech-hidpp.o
 obj-m	+= hid-logitech-wtp.o
+obj-m	+= hid-logitech-m560.o
 
 KDIR := /lib/modules/$(shell uname -r)/build
 PWD := $(shell pwd)
diff --git a/hid-logitech-m560.c b/hid-logitech-m560.c
new file mode 100644
index 0000000..5758423
--- /dev/null
+++ b/hid-logitech-m560.c
@@ -0,0 +1,378 @@
+/*
+ *  HID driver for Logitech m560 mouse
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/workqueue.h>
+#include "hid-ids.h"
+#include "hid-logitech-dj.h"
+#include "hid-logitech-hidpp.h"
+
+#define DJ_DEVICE_ID_M560 0x402d
+
+/*
+ * Logitech M560 protocol overview
+ *
+ * The Logitech M560 mouse, is designed for windows 8. When the middle and/or
+ * the sides buttons are pressed, it sends some keyboard keys events
+ * instead of buttons ones.
+ * To complicate further the things, the middle button keys sequence
+ * is different from the odd press and the even press.
+ *
+ * forward button -> Super_R
+ * backward button -> Super_L+'d' (press only)
+ * middle button -> 1st time: Alt_L+SuperL+XF86TouchpadOff (press only)
+ *                  2nd time: left-click (press only)
+ * NB: press-only means that when the button is pressed, the
+ * KeyPress/ButtonPress and KeyRelease/ButtonRelease events are generated
+ * together sequentially; instead when the button is released, no event is
+ * generated !
+ *
+ * With the command
+ *	10<xx>0a 3500af03 (where <xx> is the mouse id),
+ * the mouse reacts differently:
+ * - it never send a keyboard key event
+ * - for the three mouse button it sends:
+ *	middle button               press   11<xx>0a 3500af00...
+ *	side 1 button (forward)     press   11<xx>0a 3500b000...
+ *	side 2 button (backward)    press   11<xx>0a 3500ae00...
+ *	middle/side1/side2 button   release 11<xx>0a 35000000...
+ */
+static u8 m560_config_command[] = {0x35, 0x00, 0xaf, 0x03};
+
+struct m560_private_data {
+	union {
+		struct dj_report	dj_report;
+		u8 data[sizeof(struct dj_report)];
+	};
+	bool do_config_command;
+	struct delayed_work  work;
+	struct dj_device *djdev;
+	unsigned long packet_last_time;
+};
+
+/* how the button are mapped in the report */
+#define MOUSE_BTN_LEFT		0
+#define MOUSE_BTN_RIGHT		1
+#define MOUSE_BTN_MIDDLE	2
+#define MOUSE_BTN_WHEEL_LEFT	3
+#define MOUSE_BTN_WHEEL_RIGHT	4
+#define MOUSE_BTN_FORWARD	5
+#define MOUSE_BTN_BACKWARD	6
+
+#define CONFIG_COMMAND_TIMEOUT	(3*HZ)
+#define PACKET_TIMEOUT		(10*HZ)
+
+
+/*
+ * m560_send_config_command - send the config_command to the mouse
+ *
+ * @dev: hid device where the mouse belongs
+ *
+ * @return: >0 OK, <0 error
+ */
+static int m560_send_config_command(struct hid_device *hdev)
+{
+	struct dj_report *dj_report;
+	int retval;
+	struct dj_device *dj_device = hdev->driver_data;
+	struct hid_device *djrcv_hdev = dj_device->dj_receiver_dev->hdev;
+
+	dj_report = kzalloc(sizeof(struct dj_report), GFP_KERNEL);
+	if (!dj_report)
+		return -ENOMEM;
+
+	dj_report->report_id = REPORT_ID_HIDPP_SHORT;
+	dj_report->device_index = dj_device->device_index;
+	dj_report->report_type = 0x0a;
+
+	memcpy(dj_report->report_params, m560_config_command,
+					sizeof(m560_config_command));
+	retval = hid_hw_raw_request(djrcv_hdev,
+		dj_report->report_id,
+		(void *)dj_report, HIDPP_REPORT_SHORT_LENGTH,
+		HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+
+	kfree(dj_report);
+	return retval;
+
+}
+
+/*
+ * delayedwork_callback - handle the sending of the config_command.
+ * It schedules another sending because sometime the mouse doesn't understand
+ * the first request and returns an error. So until an ack is received, this
+ * function continue to reschedule a sending each RESET_TIMEOUT seconds
+ *
+ * @work: work_struct struct
+ */
+static void delayedwork_callback(struct work_struct *work)
+{
+	struct m560_private_data *mydata =
+		container_of(work, struct m560_private_data, work.work);
+	struct hid_device *hdev = mydata->djdev->hdev;
+
+	if (!mydata->do_config_command)
+		return;
+
+	if (schedule_delayed_work(&mydata->work, CONFIG_COMMAND_TIMEOUT) == 0) {
+		dbg_hid(
+		  "%s: did not schedule the work item, was already queued\n",
+		  __func__);
+	}
+
+	m560_send_config_command(hdev);
+}
+
+/*
+ * start_config_command - start sending config_command
+ *
+ * @mydata: pointer to private driver data
+ */
+static inline void start_config_command(struct m560_private_data *mydata)
+{
+	mydata->do_config_command = true;
+	if (schedule_delayed_work(&mydata->work, HZ/2) == 0) {
+		struct hid_device *hdev = mydata->djdev->hdev;
+		hid_err(hdev,
+		   "%s: did not schedule the work item, was already queued\n",
+		   __func__);
+	}
+}
+
+/*
+ * stop_config_command - stop sending config_command
+ *
+ * @mydata: pointer to private driver data
+ */
+static inline void stop_config_command(struct m560_private_data *mydata)
+{
+	mydata->do_config_command = false;
+}
+
+/*
+ * m560_djdevice_probe - perform the probing of the device.
+ *
+ * @hdev: hid device
+ * @id: hid device id
+ */
+static int m560_djdevice_probe(struct hid_device *hdev,
+			 const struct hid_device_id *id)
+{
+
+	int ret;
+	struct m560_private_data *mydata;
+	struct dj_device *dj_device = hdev->driver_data;
+
+	if (strcmp(hdev->name, "M560"))
+		return -ENODEV;
+
+	mydata = kzalloc(sizeof(struct m560_private_data), GFP_KERNEL);
+	if (!mydata)
+		return -ENOMEM;
+
+	mydata->djdev = dj_device;
+
+	/* force it so input_mapping doesn't pass anything */
+	mydata->do_config_command = true;
+
+	ret = hid_parse(hdev);
+	if (!ret)
+		ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+
+	/* we can't set before */
+	hid_set_drvdata(hdev, mydata);
+
+	if (ret) {
+		kfree(mydata);
+		return ret;
+	}
+	INIT_DELAYED_WORK(&mydata->work, delayedwork_callback);
+
+	start_config_command(mydata);
+
+	return 0;
+}
+
+static inline int mouse_btn_byte(int bit)
+{
+	return (bit / 8) & 0x01;
+}
+
+static inline int mouse_btn_bit(int bit)
+{
+	return 1 << (bit & 0x07);
+}
+
+static int m560_dj_raw_event(struct hid_device *hdev,
+			struct hid_report *report, u8 *data, int size)
+{
+	struct m560_private_data *mydata = hid_get_drvdata(hdev);
+	int i;
+
+	/* count the zeros; the for body is empty */
+	for (i = 1 ; i < size && data[i] == 0 ; i++) ;
+
+	/* check if the data is a mouse related report */
+	if (data[0] != REPORT_TYPE_MOUSE && data[2] != 0x0a)
+		return 1;
+
+	/*
+	 * Check if the last packet was older than PACKET_TIMEOUT
+	 * This may be a sign of a disconnection; in this case
+	 * we resend the config_command
+	 *
+	 * NOTE: in case of a disconnection, the receiver driver
+	 * send the sequence 0x01 0x00 ... 0x00 and
+	 * 0x02 0x00 ... 0x00, but these are possibile true packet
+	 * sent by the mouse (release of the left and the front button)
+	 */
+	if (mydata->packet_last_time &&
+	    (jiffies - mydata->packet_last_time) > PACKET_TIMEOUT) {
+		start_config_command(mydata);
+	}
+	mydata->packet_last_time = jiffies;
+
+	/* check if the report is the ack of the config_command */
+	if (data[0] == 0x11 && data[2] == 0x0a &&
+	    size >= (3+sizeof(m560_config_command)) &&
+	    !memcmp(data+3, m560_config_command,
+		sizeof(m560_config_command))) {
+
+			stop_config_command(mydata);
+			return true;
+	}
+
+	/*
+	 * check if the report is a mouse button sequence
+	 *
+	 * mydata->data[0] = type (0x02)
+	 * mydata->data[1..2] = buttons
+	 * mydata->data[3..5] = xy
+	 * mydata->data[6] = wheel
+	 * mydata->data[7] = horizontal wheel
+	 */
+	if (data[0] == 0x11 && data[2] == 0x0a && data[06] == 0x00) {
+		int btn, i, maxsize;
+
+		/* check if the event is a button */
+		btn = data[5];
+		if (btn != 0x00 && btn != 0xb0 && btn != 0xae && btn != 0xaf)
+			return true;
+
+		if (btn == 0xaf)
+			mydata->data[1+mouse_btn_byte(MOUSE_BTN_MIDDLE)] |=
+				mouse_btn_bit(MOUSE_BTN_MIDDLE);
+		else if (btn == 0xb0)
+			mydata->data[1+mouse_btn_byte(MOUSE_BTN_FORWARD)] |=
+				mouse_btn_bit(MOUSE_BTN_FORWARD);
+		else if (btn == 0xae)
+			mydata->data[1+mouse_btn_byte(MOUSE_BTN_BACKWARD)] |=
+				mouse_btn_bit(MOUSE_BTN_BACKWARD);
+
+		else if (btn == 0x00) {
+			mydata->data[1+mouse_btn_byte(MOUSE_BTN_MIDDLE)] &=
+				~mouse_btn_bit(MOUSE_BTN_MIDDLE);
+			mydata->data[1+mouse_btn_byte(MOUSE_BTN_FORWARD)] &=
+				~mouse_btn_bit(MOUSE_BTN_FORWARD);
+			mydata->data[1+mouse_btn_byte(MOUSE_BTN_BACKWARD)] &=
+				~mouse_btn_bit(MOUSE_BTN_BACKWARD);
+		}
+
+		/* replace the report with a standard one */
+		if (size > sizeof(mydata->data))
+			maxsize = sizeof(mydata->data);
+		else
+			maxsize = size;
+		for (i = 0 ; i < maxsize ; i++)
+			data[i] = mydata->data[i];
+
+		return 1;
+	}
+
+	/* check if the report is a "standard" mouse report */
+	if (data[0] == REPORT_TYPE_MOUSE) {
+		int i;
+
+		/* horizontal wheel handling */
+		if (data[1+mouse_btn_byte(MOUSE_BTN_WHEEL_LEFT)] &
+		    mouse_btn_bit(MOUSE_BTN_WHEEL_LEFT))
+			data[1+6] = -1;
+		if (data[1+mouse_btn_byte(MOUSE_BTN_WHEEL_RIGHT)] &
+		    mouse_btn_bit(MOUSE_BTN_WHEEL_RIGHT))
+			data[1+6] =  1;
+
+		data[1+mouse_btn_byte(MOUSE_BTN_WHEEL_LEFT)] &=
+		    ~mouse_btn_bit(MOUSE_BTN_WHEEL_LEFT);
+		data[1+mouse_btn_byte(MOUSE_BTN_WHEEL_RIGHT)] &=
+		    ~mouse_btn_bit(MOUSE_BTN_WHEEL_RIGHT);
+
+		/* copy the button status */
+		for (i = 0 ; i < 3 ; i++)
+			mydata->data[i] = data[i];
+	}
+
+	return 1;
+}
+
+/*
+ * This function performs the cleanup when the device is removed
+ */
+static void m560_djdevice_remove(struct hid_device *hdev)
+{
+	struct m560_private_data *mydata = hid_get_drvdata(hdev);
+
+	if (!mydata)
+		return;
+
+	cancel_delayed_work_sync(&mydata->work);
+	hid_hw_stop(hdev);
+	kfree(mydata);
+}
+
+/*
+ * This function avoids that any event different from the mouse ones
+ * goes to the upper level
+ */
+static int m560_djdevice_input_mapping(struct hid_device *hdev,
+		struct hid_input *hi, struct hid_field *field,
+		struct hid_usage *usage, unsigned long **bit, int *max)
+{
+	if (field->application != HID_GD_MOUSE)
+		return -1;
+	return 0;
+}
+
+static const struct hid_device_id m560_dj_device[] = {
+	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE_GENERIC,
+		USB_VENDOR_ID_LOGITECH, DJ_DEVICE_ID_M560)},
+	{}
+};
+
+MODULE_DEVICE_TABLE(hid, m560_dj_device);
+
+struct hid_driver hid_logitech_dj_device_driver_m560 = {
+	.name = "m560",
+	.id_table = m560_dj_device,
+	.probe = m560_djdevice_probe,
+	.remove = m560_djdevice_remove,
+	.input_mapping = m560_djdevice_input_mapping,
+	.raw_event = m560_dj_raw_event,
+};
+
+module_hid_driver(hid_logitech_dj_device_driver_m560)
+
+MODULE_AUTHOR("Goffredo Baroncelli <kreijack@inwind.it>");
+MODULE_DESCRIPTION("Logitech Wireless Mouse m560");
+MODULE_LICENSE("GPL");

Re: Driver for Logitech M560

[Benjamin Tissoires]

Hi Goffredo,

On Mon, Sep 1, 2014 at 3:20 PM, Goffredo Baroncelli <kreijack@inwind.it> wrote:
> Hi Benjamin,
>
> following the Nestor suggestion, I rewrote the driver for the
> mouse Logitech M560 on the basis of your work (branch "for-whot").

Just for the record. This branch is located here:
https://github.com/bentiss/hid-logitech-dj/tree/for-whot and I
*really* need to finish this work so that everything can go upstream
:(

>
> The Logitech M560 is is a mouse designed for windows 8.
> Comparing to a standard one, some buttons (the middle one and the
> two ones placed on the side) are bounded to a key combination
> instead of a classic "mouse" button.
>
> Think this mouse as a pair of mouse and keyboard. When the middle
> button is pressed the it sends a key (as keyboard)
> combination, the same for the other two side button.
> Instead the left/right/wheel work correctly.
> To complicate further the things, the middle button send a
> key combination the odd press, and another one for the even press;
> in the latter case it sends also a left click. But the worst thing
> is that no event is generated when the middle button is released.
>
> Moreover this device is a wireless mouse which uses the unifying
> receiver.
>
> I discovered that it is possible to re-configure the mouse
> sending a command (see function m560_send_config_command()).
> After this command the mouse sends some sequence when the
> buttons are pressed and/or released (see comments for
> an explanation of the mouse protocol).
>
> If the mouse is "silent" (no data is sent) for more than
> PACKET_TIMEOUT seconds (currently 10), when the next packet
> comes, the driver try to reconfigure it.
> This because it is possible that the mouse is switched-off
> and lost its setting. Se we need to reconfigure.

This just looks weird. I am pretty sure we managed to properly tackle
that for the touchpads, there should not be any difference with the
mouse. Or maybe we did not?
Anyway, calling this every timeout is not a good solution IMO.


>
> Benjamin, I have to highlight three things:
> when the dj driver detects a disconnection, it
> sends a sequence like
>
>   0x01 0x00 0x00 0x00 0x00 0x00 0x00 ...
>   0x02 0x00 0x00 0x00 0x00 0x00 0x00 ...
>
> because the mouse is both a keyboard (0x01) and
> a mouse (0x02). But this sequence is a valid one
> (when two buttons are released) due to the strangeness
> of the protocol.
>
> Can I suggest to send in these case a sequence like
>
>   <device_type> 0xff 0xff 0xff 0xff 0xff 0xff....
>
> ? I suspect that this would be less common.

Nope. This does not work. The idea behind sending the "null" report is
that this report will reset the current state of the keyboards/mice by
sending a "all buttons are now released" event. It is _not_ designed
as a marker that the device has been disconnected.

If you need this info, then another mechanism has to be used.

>
> Another thing that seemed strange to me is that report
> with ID equal 0x20 and 0x21 are handled so the packet
> are forwarded to the driver from the 3rd byte onward.

Yes. 0x20 and 0x21 are DJ reports, which encapsulate a mouse/keyboard
report coming from the DJ device.
To be properly handled by the final driver, we have to remove the
encapsulation fields so that the reports are just plain HID.

> In the others cases (this mouse send also packet with
> ID=0x11) the report id forwarded as is (without
> by-passing the first two bytes). It is the expected
> behaviour, or the code was written on the basis the
> assumption that the devices send ID=0x20/0x21 and the
> other ID are sent only by the receiver ?

No. The 0x10/0x11 reports are from the HID++ Logitech-specific
implementation, and must be forwarded as is. They have to be handled
as such by the final driver, and so, they must keep their report ID.

>
> Finally I have to point out that to work, this driver
> has to be inserted BEFORE the hid_logitech_dj
>
> In fact I had to add a file /etc/modprobe.d/hid-logitech_dj.conf
> as below
>
> install hid_logitech_dj modprobe hid_logitech_m560; sleep 5s ; modprobe --ignore-install hid_logitech_dj
>
> It is possible that the "sleep 5s" is not needed.

Hmm. This might be because there are some conflicts between your
current install and the one you just changed. I would not worry about
that. When this work will land upstream, there will be not problems.

I still did not decided if I am going to take this right now into the
github tree or not (and make a deeper review).

BTW, there is no need to send such patch to the LKML currently, you
are working against my non upstream branch. What you can do is either
setup a github clone with your patch/modifications if you want to
share them, or I can also pull this in a separate branch.

Once I'll finish the changes I want to do to hid-logitech-dj and
hid-logitech-hidpp, then we can think of merging that in my
submission.
Sorry, I still need a little bit of time to finish up this work.

Cheers,
Benjamin


> --
> gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
> Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
>
>
> ---
>
> This driver add support for the mouse Logitech m560
>
> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
>
> diff --git a/Makefile b/Makefile
> index 3016586..86157c0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,6 +1,7 @@
>  obj-m  += hid-logitech-dj.o
>  obj-m  += hid-logitech-hidpp.o
>  obj-m  += hid-logitech-wtp.o
> +obj-m  += hid-logitech-m560.o
>
>  KDIR := /lib/modules/$(shell uname -r)/build
>  PWD := $(shell pwd)
> diff --git a/hid-logitech-m560.c b/hid-logitech-m560.c
> new file mode 100644
> index 0000000..5758423
> --- /dev/null
> +++ b/hid-logitech-m560.c
> @@ -0,0 +1,378 @@
> +/*
> + *  HID driver for Logitech m560 mouse
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +#include <linux/sched.h>
> +#include <linux/workqueue.h>
> +#include "hid-ids.h"
> +#include "hid-logitech-dj.h"
> +#include "hid-logitech-hidpp.h"
> +
> +#define DJ_DEVICE_ID_M560 0x402d
> +
> +/*
> + * Logitech M560 protocol overview
> + *
> + * The Logitech M560 mouse, is designed for windows 8. When the middle and/or
> + * the sides buttons are pressed, it sends some keyboard keys events
> + * instead of buttons ones.
> + * To complicate further the things, the middle button keys sequence
> + * is different from the odd press and the even press.
> + *
> + * forward button -> Super_R
> + * backward button -> Super_L+'d' (press only)
> + * middle button -> 1st time: Alt_L+SuperL+XF86TouchpadOff (press only)
> + *                  2nd time: left-click (press only)
> + * NB: press-only means that when the button is pressed, the
> + * KeyPress/ButtonPress and KeyRelease/ButtonRelease events are generated
> + * together sequentially; instead when the button is released, no event is
> + * generated !
> + *
> + * With the command
> + *     10<xx>0a 3500af03 (where <xx> is the mouse id),
> + * the mouse reacts differently:
> + * - it never send a keyboard key event
> + * - for the three mouse button it sends:
> + *     middle button               press   11<xx>0a 3500af00...
> + *     side 1 button (forward)     press   11<xx>0a 3500b000...
> + *     side 2 button (backward)    press   11<xx>0a 3500ae00...
> + *     middle/side1/side2 button   release 11<xx>0a 35000000...
> + */
> +static u8 m560_config_command[] = {0x35, 0x00, 0xaf, 0x03};
> +
> +struct m560_private_data {
> +       union {
> +               struct dj_report        dj_report;
> +               u8 data[sizeof(struct dj_report)];
> +       };
> +       bool do_config_command;
> +       struct delayed_work  work;
> +       struct dj_device *djdev;
> +       unsigned long packet_last_time;
> +};
> +
> +/* how the button are mapped in the report */
> +#define MOUSE_BTN_LEFT         0
> +#define MOUSE_BTN_RIGHT                1
> +#define MOUSE_BTN_MIDDLE       2
> +#define MOUSE_BTN_WHEEL_LEFT   3
> +#define MOUSE_BTN_WHEEL_RIGHT  4
> +#define MOUSE_BTN_FORWARD      5
> +#define MOUSE_BTN_BACKWARD     6
> +
> +#define CONFIG_COMMAND_TIMEOUT (3*HZ)
> +#define PACKET_TIMEOUT         (10*HZ)
> +
> +
> +/*
> + * m560_send_config_command - send the config_command to the mouse
> + *
> + * @dev: hid device where the mouse belongs
> + *
> + * @return: >0 OK, <0 error
> + */
> +static int m560_send_config_command(struct hid_device *hdev)
> +{
> +       struct dj_report *dj_report;
> +       int retval;
> +       struct dj_device *dj_device = hdev->driver_data;
> +       struct hid_device *djrcv_hdev = dj_device->dj_receiver_dev->hdev;
> +
> +       dj_report = kzalloc(sizeof(struct dj_report), GFP_KERNEL);
> +       if (!dj_report)
> +               return -ENOMEM;
> +
> +       dj_report->report_id = REPORT_ID_HIDPP_SHORT;
> +       dj_report->device_index = dj_device->device_index;
> +       dj_report->report_type = 0x0a;
> +
> +       memcpy(dj_report->report_params, m560_config_command,
> +                                       sizeof(m560_config_command));
> +       retval = hid_hw_raw_request(djrcv_hdev,
> +               dj_report->report_id,
> +               (void *)dj_report, HIDPP_REPORT_SHORT_LENGTH,
> +               HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
> +
> +       kfree(dj_report);
> +       return retval;
> +
> +}
> +
> +/*
> + * delayedwork_callback - handle the sending of the config_command.
> + * It schedules another sending because sometime the mouse doesn't understand
> + * the first request and returns an error. So until an ack is received, this
> + * function continue to reschedule a sending each RESET_TIMEOUT seconds
> + *
> + * @work: work_struct struct
> + */
> +static void delayedwork_callback(struct work_struct *work)
> +{
> +       struct m560_private_data *mydata =
> +               container_of(work, struct m560_private_data, work.work);
> +       struct hid_device *hdev = mydata->djdev->hdev;
> +
> +       if (!mydata->do_config_command)
> +               return;
> +
> +       if (schedule_delayed_work(&mydata->work, CONFIG_COMMAND_TIMEOUT) == 0) {
> +               dbg_hid(
> +                 "%s: did not schedule the work item, was already queued\n",
> +                 __func__);
> +       }
> +
> +       m560_send_config_command(hdev);
> +}
> +
> +/*
> + * start_config_command - start sending config_command
> + *
> + * @mydata: pointer to private driver data
> + */
> +static inline void start_config_command(struct m560_private_data *mydata)
> +{
> +       mydata->do_config_command = true;
> +       if (schedule_delayed_work(&mydata->work, HZ/2) == 0) {
> +               struct hid_device *hdev = mydata->djdev->hdev;
> +               hid_err(hdev,
> +                  "%s: did not schedule the work item, was already queued\n",
> +                  __func__);
> +       }
> +}
> +
> +/*
> + * stop_config_command - stop sending config_command
> + *
> + * @mydata: pointer to private driver data
> + */
> +static inline void stop_config_command(struct m560_private_data *mydata)
> +{
> +       mydata->do_config_command = false;
> +}
> +
> +/*
> + * m560_djdevice_probe - perform the probing of the device.
> + *
> + * @hdev: hid device
> + * @id: hid device id
> + */
> +static int m560_djdevice_probe(struct hid_device *hdev,
> +                        const struct hid_device_id *id)
> +{
> +
> +       int ret;
> +       struct m560_private_data *mydata;
> +       struct dj_device *dj_device = hdev->driver_data;
> +
> +       if (strcmp(hdev->name, "M560"))
> +               return -ENODEV;
> +
> +       mydata = kzalloc(sizeof(struct m560_private_data), GFP_KERNEL);
> +       if (!mydata)
> +               return -ENOMEM;
> +
> +       mydata->djdev = dj_device;
> +
> +       /* force it so input_mapping doesn't pass anything */
> +       mydata->do_config_command = true;
> +
> +       ret = hid_parse(hdev);
> +       if (!ret)
> +               ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> +
> +       /* we can't set before */
> +       hid_set_drvdata(hdev, mydata);
> +
> +       if (ret) {
> +               kfree(mydata);
> +               return ret;
> +       }
> +       INIT_DELAYED_WORK(&mydata->work, delayedwork_callback);
> +
> +       start_config_command(mydata);
> +
> +       return 0;
> +}
> +
> +static inline int mouse_btn_byte(int bit)
> +{
> +       return (bit / 8) & 0x01;
> +}
> +
> +static inline int mouse_btn_bit(int bit)
> +{
> +       return 1 << (bit & 0x07);
> +}
> +
> +static int m560_dj_raw_event(struct hid_device *hdev,
> +                       struct hid_report *report, u8 *data, int size)
> +{
> +       struct m560_private_data *mydata = hid_get_drvdata(hdev);
> +       int i;
> +
> +       /* count the zeros; the for body is empty */
> +       for (i = 1 ; i < size && data[i] == 0 ; i++) ;
> +
> +       /* check if the data is a mouse related report */
> +       if (data[0] != REPORT_TYPE_MOUSE && data[2] != 0x0a)
> +               return 1;
> +
> +       /*
> +        * Check if the last packet was older than PACKET_TIMEOUT
> +        * This may be a sign of a disconnection; in this case
> +        * we resend the config_command
> +        *
> +        * NOTE: in case of a disconnection, the receiver driver
> +        * send the sequence 0x01 0x00 ... 0x00 and
> +        * 0x02 0x00 ... 0x00, but these are possibile true packet
> +        * sent by the mouse (release of the left and the front button)
> +        */
> +       if (mydata->packet_last_time &&
> +           (jiffies - mydata->packet_last_time) > PACKET_TIMEOUT) {
> +               start_config_command(mydata);
> +       }
> +       mydata->packet_last_time = jiffies;
> +
> +       /* check if the report is the ack of the config_command */
> +       if (data[0] == 0x11 && data[2] == 0x0a &&
> +           size >= (3+sizeof(m560_config_command)) &&
> +           !memcmp(data+3, m560_config_command,
> +               sizeof(m560_config_command))) {
> +
> +                       stop_config_command(mydata);
> +                       return true;
> +       }
> +
> +       /*
> +        * check if the report is a mouse button sequence
> +        *
> +        * mydata->data[0] = type (0x02)
> +        * mydata->data[1..2] = buttons
> +        * mydata->data[3..5] = xy
> +        * mydata->data[6] = wheel
> +        * mydata->data[7] = horizontal wheel
> +        */
> +       if (data[0] == 0x11 && data[2] == 0x0a && data[06] == 0x00) {
> +               int btn, i, maxsize;
> +
> +               /* check if the event is a button */
> +               btn = data[5];
> +               if (btn != 0x00 && btn != 0xb0 && btn != 0xae && btn != 0xaf)
> +                       return true;
> +
> +               if (btn == 0xaf)
> +                       mydata->data[1+mouse_btn_byte(MOUSE_BTN_MIDDLE)] |=
> +                               mouse_btn_bit(MOUSE_BTN_MIDDLE);
> +               else if (btn == 0xb0)
> +                       mydata->data[1+mouse_btn_byte(MOUSE_BTN_FORWARD)] |=
> +                               mouse_btn_bit(MOUSE_BTN_FORWARD);
> +               else if (btn == 0xae)
> +                       mydata->data[1+mouse_btn_byte(MOUSE_BTN_BACKWARD)] |=
> +                               mouse_btn_bit(MOUSE_BTN_BACKWARD);
> +
> +               else if (btn == 0x00) {
> +                       mydata->data[1+mouse_btn_byte(MOUSE_BTN_MIDDLE)] &=
> +                               ~mouse_btn_bit(MOUSE_BTN_MIDDLE);
> +                       mydata->data[1+mouse_btn_byte(MOUSE_BTN_FORWARD)] &=
> +                               ~mouse_btn_bit(MOUSE_BTN_FORWARD);
> +                       mydata->data[1+mouse_btn_byte(MOUSE_BTN_BACKWARD)] &=
> +                               ~mouse_btn_bit(MOUSE_BTN_BACKWARD);
> +               }
> +
> +               /* replace the report with a standard one */
> +               if (size > sizeof(mydata->data))
> +                       maxsize = sizeof(mydata->data);
> +               else
> +                       maxsize = size;
> +               for (i = 0 ; i < maxsize ; i++)
> +                       data[i] = mydata->data[i];
> +
> +               return 1;
> +       }
> +
> +       /* check if the report is a "standard" mouse report */
> +       if (data[0] == REPORT_TYPE_MOUSE) {
> +               int i;
> +
> +               /* horizontal wheel handling */
> +               if (data[1+mouse_btn_byte(MOUSE_BTN_WHEEL_LEFT)] &
> +                   mouse_btn_bit(MOUSE_BTN_WHEEL_LEFT))
> +                       data[1+6] = -1;
> +               if (data[1+mouse_btn_byte(MOUSE_BTN_WHEEL_RIGHT)] &
> +                   mouse_btn_bit(MOUSE_BTN_WHEEL_RIGHT))
> +                       data[1+6] =  1;
> +
> +               data[1+mouse_btn_byte(MOUSE_BTN_WHEEL_LEFT)] &=
> +                   ~mouse_btn_bit(MOUSE_BTN_WHEEL_LEFT);
> +               data[1+mouse_btn_byte(MOUSE_BTN_WHEEL_RIGHT)] &=
> +                   ~mouse_btn_bit(MOUSE_BTN_WHEEL_RIGHT);
> +
> +               /* copy the button status */
> +               for (i = 0 ; i < 3 ; i++)
> +                       mydata->data[i] = data[i];
> +       }
> +
> +       return 1;
> +}
> +
> +/*
> + * This function performs the cleanup when the device is removed
> + */
> +static void m560_djdevice_remove(struct hid_device *hdev)
> +{
> +       struct m560_private_data *mydata = hid_get_drvdata(hdev);
> +
> +       if (!mydata)
> +               return;
> +
> +       cancel_delayed_work_sync(&mydata->work);
> +       hid_hw_stop(hdev);
> +       kfree(mydata);
> +}
> +
> +/*
> + * This function avoids that any event different from the mouse ones
> + * goes to the upper level
> + */
> +static int m560_djdevice_input_mapping(struct hid_device *hdev,
> +               struct hid_input *hi, struct hid_field *field,
> +               struct hid_usage *usage, unsigned long **bit, int *max)
> +{
> +       if (field->application != HID_GD_MOUSE)
> +               return -1;
> +       return 0;
> +}
> +
> +static const struct hid_device_id m560_dj_device[] = {
> +       { HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE_GENERIC,
> +               USB_VENDOR_ID_LOGITECH, DJ_DEVICE_ID_M560)},
> +       {}
> +};
> +
> +MODULE_DEVICE_TABLE(hid, m560_dj_device);
> +
> +struct hid_driver hid_logitech_dj_device_driver_m560 = {
> +       .name = "m560",
> +       .id_table = m560_dj_device,
> +       .probe = m560_djdevice_probe,
> +       .remove = m560_djdevice_remove,
> +       .input_mapping = m560_djdevice_input_mapping,
> +       .raw_event = m560_dj_raw_event,
> +};
> +
> +module_hid_driver(hid_logitech_dj_device_driver_m560)
> +
> +MODULE_AUTHOR("Goffredo Baroncelli <kreijack@inwind.it>");
> +MODULE_DESCRIPTION("Logitech Wireless Mouse m560");
> +MODULE_LICENSE("GPL");
>
>
>

Re: Driver for Logitech M560

[Goffredo Baroncelli]

On 09/03/2014 11:36 PM, Benjamin Tissoires wrote:
> Hi Goffredo,
> 
> On Mon, Sep 1, 2014 at 3:20 PM, Goffredo Baroncelli <kreijack@inwind.it> wrote:
>> Hi Benjamin,
>>
>> following the Nestor suggestion, I rewrote the driver for the
>> mouse Logitech M560 on the basis of your work (branch "for-whot").
> 
> Just for the record. This branch is located here:
> https://github.com/bentiss/hid-logitech-dj/tree/for-whot and I
> *really* need to finish this work so that everything can go upstream
> :(

:-) For me this is not a problem. I solved my issue (I made a dkms
package for this mouse), but I want to share my effort..

> 
>>
>> The Logitech M560 is is a mouse designed for windows 8.
>> Comparing to a standard one, some buttons (the middle one and the
>> two ones placed on the side) are bounded to a key combination
>> instead of a classic "mouse" button.
>>
>> Think this mouse as a pair of mouse and keyboard. When the middle
>> button is pressed the it sends a key (as keyboard)
>> combination, the same for the other two side button.
>> Instead the left/right/wheel work correctly.
>> To complicate further the things, the middle button send a
>> key combination the odd press, and another one for the even press;
>> in the latter case it sends also a left click. But the worst thing
>> is that no event is generated when the middle button is released.
>>
>> Moreover this device is a wireless mouse which uses the unifying
>> receiver.
>>
>> I discovered that it is possible to re-configure the mouse
>> sending a command (see function m560_send_config_command()).
>> After this command the mouse sends some sequence when the
>> buttons are pressed and/or released (see comments for
>> an explanation of the mouse protocol).
>>
>> If the mouse is "silent" (no data is sent) for more than
>> PACKET_TIMEOUT seconds (currently 10), when the next packet
>> comes, the driver try to reconfigure it.
>> This because it is possible that the mouse is switched-off
>> and lost its setting. Se we need to reconfigure.
> 
> This just looks weird. I am pretty sure we managed to properly tackle
> that for the touchpads, there should not be any difference with the
> mouse. Or maybe we did not?
> Anyway, calling this every timeout is not a good solution IMO.

I looked at the wtp source, and I was not able to find anything.
Anyway I will remove the reset-by-timeout; eventually I split this
in another patch for further development
> 
> 
>>
>> Benjamin, I have to highlight three things:
>> when the dj driver detects a disconnection, it
>> sends a sequence like
>>
>>   0x01 0x00 0x00 0x00 0x00 0x00 0x00 ...
>>   0x02 0x00 0x00 0x00 0x00 0x00 0x00 ...
>>
>> because the mouse is both a keyboard (0x01) and
>> a mouse (0x02). But this sequence is a valid one
>> (when two buttons are released) due to the strangeness
>> of the protocol.
>>
>> Can I suggest to send in these case a sequence like
>>
>>   <device_type> 0xff 0xff 0xff 0xff 0xff 0xff....
>>
>> ? I suspect that this would be less common.
> 
> Nope. This does not work. The idea behind sending the "null" report is
> that this report will reset the current state of the keyboards/mice by
> sending a "all buttons are now released" event. It is _not_ designed
> as a marker that the device has been disconnected.
> 
> If you need this info, then another mechanism has to be used.

Have you any suggestions ?

> 
>>
>> Another thing that seemed strange to me is that report
>> with ID equal 0x20 and 0x21 are handled so the packet
>> are forwarded to the driver from the 3rd byte onward.
> 
> Yes. 0x20 and 0x21 are DJ reports, which encapsulate a mouse/keyboard
> report coming from the DJ device.
> To be properly handled by the final driver, we have to remove the
> encapsulation fields so that the reports are just plain HID.
> 
>> In the others cases (this mouse send also packet with
>> ID=0x11) the report id forwarded as is (without
>> by-passing the first two bytes). It is the expected
>> behaviour, or the code was written on the basis the
>> assumption that the devices send ID=0x20/0x21 and the
>> other ID are sent only by the receiver ?
> 
> No. The 0x10/0x11 reports are from the HID++ Logitech-specific
> implementation, and must be forwarded as is. They have to be handled
> as such by the final driver, and so, they must keep their report ID.

Ok, I don't have any problem about that. I want to be sure that it
is intentionally and not an hole discovered due to the M560 mouse 
protocol.

> 
>>
>> Finally I have to point out that to work, this driver
>> has to be inserted BEFORE the hid_logitech_dj
>>
>> In fact I had to add a file /etc/modprobe.d/hid-logitech_dj.conf
>> as below
>>
>> install hid_logitech_dj modprobe hid_logitech_m560; sleep 5s ; modprobe --ignore-install hid_logitech_dj
>>
>> It is possible that the "sleep 5s" is not needed.
> 
> Hmm. This might be because there are some conflicts between your
> current install and the one you just changed. I would not worry about
> that. When this work will land upstream, there will be not problems.
> 
> I still did not decided if I am going to take this right now into the
> github tree or not (and make a deeper review).
> 
> BTW, there is no need to send such patch to the LKML currently, you
> are working against my non upstream branch. What you can do is either
> setup a github clone with your patch/modifications if you want to
> share them, or I can also pull this in a separate branch.

Ok, I will prepare (I hope this week-end) a github repo for a pull.
Let me to remove the reset-by-timeout code.

> 
> Once I'll finish the changes I want to do to hid-logitech-dj and
> hid-logitech-hidpp, then we can think of merging that in my
> submission.
> Sorry, I still need a little bit of time to finish up this work.

Ok, thanks

> Cheers,
> Benjamin
> 
[... cut ... cut ... ]

G.Baroncelli

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: Driver for Logitech M560

[Antonio Ospite]

On Fri, 05 Sep 2014 19:47:44 +0200
Goffredo Baroncelli <kreijack@inwind.it> wrote:

> On 09/03/2014 11:36 PM, Benjamin Tissoires wrote:
> > Hi Goffredo,
> > 
> > On Mon, Sep 1, 2014 at 3:20 PM, Goffredo Baroncelli <kreijack@inwind.it> wrote:
> >> Hi Benjamin,
> >>
> >> following the Nestor suggestion, I rewrote the driver for the
> >> mouse Logitech M560 on the basis of your work (branch "for-whot").
> > 
> > Just for the record. This branch is located here:
> > https://github.com/bentiss/hid-logitech-dj/tree/for-whot and I
> > *really* need to finish this work so that everything can go upstream
> > :(
>

Hi Benjamin, any news about upstreaming this work?

> :-) For me this is not a problem. I solved my issue (I made a dkms
> package for this mouse), but I want to share my effort..
> 

Goffredo, is this the latest version of your dkms-enabled external
module?
https://github.com/kreijack/hid-logitech-dj/tree/m560-dkms

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: Driver for Logitech M560

[Goffredo Baroncelli]

On 2015-04-09 14:41, Antonio Ospite wrote:
> On Fri, 05 Sep 2014 19:47:44 +0200
> Goffredo Baroncelli <kreijack@inwind.it> wrote:
> 
>> On 09/03/2014 11:36 PM, Benjamin Tissoires wrote:
>>> Hi Goffredo,
>>>
>>> On Mon, Sep 1, 2014 at 3:20 PM, Goffredo Baroncelli <kreijack@inwind.it> wrote:
>>>> Hi Benjamin,
>>>>
>>>> following the Nestor suggestion, I rewrote the driver for the
>>>> mouse Logitech M560 on the basis of your work (branch "for-whot").
>>>
>>> Just for the record. This branch is located here:
>>> https://github.com/bentiss/hid-logitech-dj/tree/for-whot and I
>>> *really* need to finish this work so that everything can go upstream
>>> :(
>>
> 
> Hi Benjamin, any news about upstreaming this work?
> 
>> :-) For me this is not a problem. I solved my issue (I made a dkms
>> package for this mouse), but I want to share my effort..
>>
> 
> Goffredo, is this the latest version of your dkms-enabled external
> module?
> https://github.com/kreijack/hid-logitech-dj/tree/m560-dkms

Hi Antonio,

I was never capable to make dkms working; if you have got it, please geve me a patch...
Anyway the most update branch is m560-devel, but you have to insert by hand the modules..

BR
Goffredo



> 
> Thanks,
>    Antonio
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: Driver for Logitech M560

[Benjamin Tissoires]

Hi Antonio,

On Thu, Apr 9, 2015 at 8:41 AM, Antonio Ospite <ao2@ao2.it> wrote:
> On Fri, 05 Sep 2014 19:47:44 +0200
> Goffredo Baroncelli <kreijack@inwind.it> wrote:
>
>> On 09/03/2014 11:36 PM, Benjamin Tissoires wrote:
>> > Hi Goffredo,
>> >
>> > On Mon, Sep 1, 2014 at 3:20 PM, Goffredo Baroncelli <kreijack@inwind.it> wrote:
>> >> Hi Benjamin,
>> >>
>> >> following the Nestor suggestion, I rewrote the driver for the
>> >> mouse Logitech M560 on the basis of your work (branch "for-whot").
>> >
>> > Just for the record. This branch is located here:
>> > https://github.com/bentiss/hid-logitech-dj/tree/for-whot and I
>> > *really* need to finish this work so that everything can go upstream
>> > :(
>>
>
> Hi Benjamin, any news about upstreaming this work?

The hidpp / raw touchpad mode is already upstream since the v3.19 kernel. \o/

We just need to port Goffredo's changes now and push them too.
There has been some differences since this branch was published. The
main is that there is no more special subdrivers in several files,
everything is handled in hidpp. I think the logic behind is somewhat
the same so it should not be too much a problem to do the work.

Cheers,
Benjamin

>
>> :-) For me this is not a problem. I solved my issue (I made a dkms
>> package for this mouse), but I want to share my effort..
>>
>
> Goffredo, is this the latest version of your dkms-enabled external
> module?
> https://github.com/kreijack/hid-logitech-dj/tree/m560-dkms
>
> Thanks,
>    Antonio
>
> --
> Antonio Ospite
> http://ao2.it
>
> A: Because it messes up the order in which people normally read text.
>    See http://en.wikipedia.org/wiki/Posting_style
> Q: Why is top-posting such a bad thing?

Re: Driver for Logitech M560

[Goffredo Baroncelli]

Hi,

after the Antonio's mail, I updated my work on the latest kernel (3.19.3); this work for me, but I am sure that I made some mistakes, so please consider this a beta.
I added a new device class (M560), and I put some hooks to process the raw data.

I used the following "quirks":
- HIDPP_QUIRK_DELAYED_INIT
- HIDPP_QUIRK_MULTI_INPUT

these work (when the mouse is connected, the configuration message is sent), but I am not sure if this is correct.
Anyway in the past I noticed some false spurious reconnections, and re-sending the configuration time to time made the mouse not-responsive for few tens of second (it seems a low value, but I noticed it); so I am not sure if I will leave the "reconfiguration" on reconnection.

Comments are welcome.

BR
G.Baroncelli


diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index a93cefe..4d907d6 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -35,6 +35,7 @@ MODULE_AUTHOR("Nestor Lopez Casado <nlopezcasad@logitech.com>");
 #define HIDPP_REPORT_LONG_LENGTH		20
 
 #define HIDPP_QUIRK_CLASS_WTP			BIT(0)
+#define HIDPP_QUIRK_CLASS_M560			BIT(1)
 
 /* bits 1..20 are reserved for classes */
 #define HIDPP_QUIRK_DELAYED_INIT		BIT(21)
@@ -925,6 +926,225 @@ static void wtp_connect(struct hid_device *hdev, bool connected)
 }
 
 /* -------------------------------------------------------------------------- */
+/* Logitech M560 devices                                                     */
+/* -------------------------------------------------------------------------- */
+
+/*
+ * Logitech M560 protocol overview
+ *
+ * The Logitech M560 mouse, is designed for windows 8. When the middle and/or
+ * the sides buttons are pressed, it sends some keyboard keys events
+ * instead of buttons ones.
+ * To complicate further the things, the middle button keys sequence
+ * is different from the odd press and the even press.
+ *
+ * forward button -> Super_R
+ * backward button -> Super_L+'d' (press only)
+ * middle button -> 1st time: Alt_L+SuperL+XF86TouchpadOff (press only)
+ *                  2nd time: left-click (press only)
+ * NB: press-only means that when the button is pressed, the
+ * KeyPress/ButtonPress and KeyRelease/ButtonRelease events are generated
+ * together sequentially; instead when the button is released, no event is
+ * generated !
+ *
+ * With the command
+ *	10<xx>0a 3500af03 (where <xx> is the mouse id),
+ * the mouse reacts differently:
+ * - it never send a keyboard key event
+ * - for the three mouse button it sends:
+ *	middle button               press   11<xx>0a 3500af00...
+ *	side 1 button (forward)     press   11<xx>0a 3500b000...
+ *	side 2 button (backward)    press   11<xx>0a 3500ae00...
+ *	middle/side1/side2 button   release 11<xx>0a 35000000...
+ */
+static u8 m560_config_command[] = {0x35, 0x00, 0xaf, 0x03};
+
+struct m560_private_data {
+	u8 prev_data[30]; // FIXME: select a right size
+	int btn_middle:1;
+	int btn_forward:1;
+	int btn_backward:1;
+};
+
+/* how the button are mapped in the report */
+#define MOUSE_BTN_LEFT		0
+#define MOUSE_BTN_RIGHT		1
+#define MOUSE_BTN_MIDDLE	2
+#define MOUSE_BTN_WHEEL_LEFT	3
+#define MOUSE_BTN_WHEEL_RIGHT	4
+#define MOUSE_BTN_FORWARD	5
+#define MOUSE_BTN_BACKWARD	6
+
+/*
+ * m560_priv_data - helper to convert from hidpp_device to m560_private_data
+ *
+ * @hdev: hid device
+ *
+ * @return: return m560_private_data if available, NULL otherwise
+ */
+static inline struct m560_private_data *m560_priv_data(struct hid_device *hdev)
+{
+	struct hidpp_device *hidpp_dev = hid_get_drvdata(hdev);
+	return hidpp_dev ? hidpp_dev->private_data : NULL;
+}
+
+/*
+ * m560_send_config_command - send the config_command to the mouse
+ *
+ * @dev: hid device where the mouse belongs
+ *
+ * @return: 0 OK
+ */
+static int m560_send_config_command(struct hid_device *hdev) {
+	struct hidpp_report response;
+	struct hidpp_device *hidpp_dev = hid_get_drvdata(hdev);
+	int ret;
+
+	ret = hidpp_send_rap_command_sync(
+		hidpp_dev,
+		REPORT_ID_HIDPP_SHORT,
+		0x0a,
+		m560_config_command[0],
+		m560_config_command+1,
+		sizeof(m560_config_command)-1,
+		&response
+	);
+
+	return ret;
+}
+
+static int m560_allocate(struct hid_device *hdev)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+	struct m560_private_data *d;
+
+	d = devm_kzalloc(&hdev->dev, sizeof(struct m560_private_data),
+			GFP_KERNEL);
+	if (!d)
+		return -ENOMEM;
+
+	hidpp->private_data = d;
+	//d->hidpp_dev = hidpp;
+	//d->hid_dev = hdev;
+
+	return 0;
+};
+
+static inline void set_btn_bit(u8 *data, int bit)
+{
+	int bytenr = bit / 8;
+	int bitmask = 1 << (bit & 0x07);
+
+	data[bytenr] |= bitmask;
+}
+
+static inline int get_btn_bit(u8 *data, int bit)
+{
+	int bytenr = bit / 8;
+	int bitmask = 1 << (bit & 0x07);
+
+	return !!(data[bytenr] & bitmask);
+}
+
+static inline void clear_btn_bit(u8 *data, int bit)
+{
+	int bytenr = bit / 8;
+	int bitmask = 1 << (bit & 0x07);
+
+	data[bytenr] &= ~bitmask;
+}
+
+static int m560_raw_event(struct hid_device *hdev, u8 *data, int size)
+{
+	struct m560_private_data *mydata = m560_priv_data(hdev);
+
+	/* check if the data is a mouse related report */
+	if (data[0] != 0x02 && data[2] != 0x0a)
+		return 1;
+
+	/* check if the report is the ack of the config_command */
+	if (data[0] == 0x11 && data[2] == 0x0a &&
+	    size >= (3+sizeof(m560_config_command)) &&
+	    !memcmp(data+3, m560_config_command,
+		sizeof(m560_config_command))) {
+			return true;
+	}
+
+	if (data[0] == 0x11 && data[2] == 0x0a && data[06] == 0x00) {
+		/*
+		 * m560 mouse button report
+		 *
+		 * data[0] = 0x11
+		 * data[1] = deviceid
+		 * data[2] = 0x0a
+		 * data[5] = button (0xaf->middle, 0xb0->forward,
+		 * 		     0xaf ->backward, 0x00->release all)
+		 * data[6] = 0x00
+		 */
+
+		int btn, i, maxsize;
+
+		/* check if the event is a button */
+		btn = data[5];
+		if (btn != 0x00 && btn != 0xb0 && btn != 0xae && btn != 0xaf)
+			return true;
+
+		if (btn == 0xaf)
+			mydata->btn_middle = 1;
+		else if (btn == 0xb0)
+			mydata->btn_forward = 1;
+		else if (btn == 0xae)
+			mydata->btn_backward = 1;
+		else if (btn == 0x00) {
+			mydata->btn_backward = 0;
+			mydata->btn_forward = 0;
+			mydata->btn_middle = 0;
+		}
+
+		/* replace the report with the old one */
+		if (size > sizeof(mydata->prev_data))
+			maxsize = sizeof(mydata->prev_data);
+		else
+			maxsize = size;
+		for (i = 0 ; i < maxsize ; i++)
+			data[i] = mydata->prev_data[i];
+
+	} else if (data[0] == 0x02) {
+		/*
+		 * standard mouse report
+		 *
+		 * data[0] = type (0x02)
+		 * data[1..2] = buttons
+		 * data[3..5] = xy
+		 * data[6] = wheel
+		 * data[7] = horizontal wheel
+		 */
+
+		/* horizontal wheel handling */
+		if (get_btn_bit(data+1,MOUSE_BTN_WHEEL_LEFT))
+			data[1+6] = -1;
+		if (get_btn_bit(data+1,MOUSE_BTN_WHEEL_RIGHT))
+			data[1+6] =  1;
+
+		clear_btn_bit(data+1, MOUSE_BTN_WHEEL_LEFT);
+		clear_btn_bit(data+1, MOUSE_BTN_WHEEL_RIGHT);
+
+		/* copy the type and buttons status */
+		memcpy(mydata->prev_data, data, 3);
+	}
+
+	/* add the extra buttons */
+	if (mydata->btn_middle)
+		set_btn_bit(data+1, MOUSE_BTN_MIDDLE);
+	if (mydata->btn_forward)
+		set_btn_bit(data+1, MOUSE_BTN_FORWARD);
+	if (mydata->btn_backward)
+		set_btn_bit(data+1, MOUSE_BTN_BACKWARD);
+
+	return 1;
+}
+
+/* -------------------------------------------------------------------------- */
 /* Generic HID++ devices                                                      */
 /* -------------------------------------------------------------------------- */
 
@@ -936,6 +1156,9 @@ static int hidpp_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		return wtp_input_mapping(hdev, hi, field, usage, bit, max);
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560 &&
+		field->application != HID_GD_MOUSE)
+		                return -1;
 
 	return 0;
 }
@@ -998,6 +1221,8 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
 
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		return wtp_raw_event(hidpp->hid_dev, data, size);
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
+		return m560_raw_event(hidpp->hid_dev, data, size);
 
 	return 0;
 }
@@ -1026,6 +1251,8 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
 
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		return wtp_raw_event(hdev, data, size);
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
+		return m560_raw_event(hidpp->hid_dev, data, size);
 
 	return 0;
 }
@@ -1100,6 +1327,8 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		wtp_connect(hdev, connected);
+	if ((hidpp->quirks & HIDPP_QUIRK_CLASS_M560) && connected)
+		m560_send_config_command(hdev);
 
 	if (!connected || hidpp->delayed_input)
 		return;
@@ -1164,6 +1393,11 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		if (ret)
 			goto wtp_allocate_fail;
 	}
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) {
+		ret = m560_allocate(hdev);
+		if (ret)
+			goto wtp_allocate_fail;
+	}
 
 	INIT_WORK(&hidpp->work, delayed_work_cb);
 	mutex_init(&hidpp->send_mutex);
@@ -1240,6 +1474,7 @@ static void hidpp_remove(struct hid_device *hdev)
 	cancel_work_sync(&hidpp->work);
 	mutex_destroy(&hidpp->send_mutex);
 	hid_hw_stop(hdev);
+
 }
 
 static const struct hid_device_id hidpp_devices[] = {
@@ -1261,6 +1496,12 @@ static const struct hid_device_id hidpp_devices[] = {
 		USB_VENDOR_ID_LOGITECH, 0x4102),
 	  .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_MULTI_INPUT |
 			 HIDPP_QUIRK_CLASS_WTP },
+	{ /* Mouse logitech M560 */
+	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
+		USB_VENDOR_ID_LOGITECH, 0x402d),
+	  .driver_data = HIDPP_QUIRK_CLASS_M560 | HIDPP_QUIRK_DELAYED_INIT |
+			 HIDPP_QUIRK_MULTI_INPUT
+	},
 
 	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},



On 2015-04-09 19:35, Benjamin Tissoires wrote:
> Hi Antonio,
> 
> On Thu, Apr 9, 2015 at 8:41 AM, Antonio Ospite <ao2@ao2.it> wrote:
>> On Fri, 05 Sep 2014 19:47:44 +0200
>> Goffredo Baroncelli <kreijack@inwind.it> wrote:
>>
>>> On 09/03/2014 11:36 PM, Benjamin Tissoires wrote:
>>>> Hi Goffredo,
>>>>
>>>> On Mon, Sep 1, 2014 at 3:20 PM, Goffredo Baroncelli <kreijack@inwind.it> wrote:
>>>>> Hi Benjamin,
>>>>>
>>>>> following the Nestor suggestion, I rewrote the driver for the
>>>>> mouse Logitech M560 on the basis of your work (branch "for-whot").
>>>>
>>>> Just for the record. This branch is located here:
>>>> https://github.com/bentiss/hid-logitech-dj/tree/for-whot and I
>>>> *really* need to finish this work so that everything can go upstream
>>>> :(
>>>
>>
>> Hi Benjamin, any news about upstreaming this work?
> 
> The hidpp / raw touchpad mode is already upstream since the v3.19 kernel. \o/
> 
> We just need to port Goffredo's changes now and push them too.
> There has been some differences since this branch was published. The
> main is that there is no more special subdrivers in several files,
> everything is handled in hidpp. I think the logic behind is somewhat
> the same so it should not be too much a problem to do the work.
> 
> Cheers,
> Benjamin
> 
>>
>>> :-) For me this is not a problem. I solved my issue (I made a dkms
>>> package for this mouse), but I want to share my effort..
>>>
>>
>> Goffredo, is this the latest version of your dkms-enabled external
>> module?
>> https://github.com/kreijack/hid-logitech-dj/tree/m560-dkms
>>
>> Thanks,
>>    Antonio
>>
>> --
>> Antonio Ospite
>> http://ao2.it
>>
>> A: Because it messes up the order in which people normally read text.
>>    See http://en.wikipedia.org/wiki/Posting_style
>> Q: Why is top-posting such a bad thing?
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: Driver for Logitech M560

[Benjamin Tissoires]

On Fri, Apr 10, 2015 at 2:56 PM, Goffredo Baroncelli <kreijack@inwind.it> wrote:
> Hi,
>
> after the Antonio's mail, I updated my work on the latest kernel (3.19.3); this work for me, but I am sure that I made some mistakes, so please consider this a beta.

Thanks for the patch. I have some comments, please see inline.

> I added a new device class (M560), and I put some hooks to process the raw data.
>
> I used the following "quirks":
> - HIDPP_QUIRK_DELAYED_INIT
> - HIDPP_QUIRK_MULTI_INPUT

I don't think you really need HIDPP_QUIRK_MULTI_INPUT. If the mouse
has only one input node (or you want to send only on the input node),
then you don't need to keep the keyboard node.

[10 min later]

OK, after reviewing the code, I see why you kept
HIDPP_QUIRK_MULTI_INPUT. I am not entirely sure what is the best
solution: keeping it and rely on hid-input for the parsing/allocating
of regular mouse events and having .raw_event() tampering with the raw
report, or just trash hid-input and process entirely the incoming
report...

>
> these work (when the mouse is connected, the configuration message is sent), but I am not sure if this is correct.
> Anyway in the past I noticed some false spurious reconnections, and re-sending the configuration time to time made the mouse not-responsive for few tens of second (it seems a low value, but I noticed it); so I am not sure if I will leave the "reconfiguration" on reconnection.
>
> Comments are welcome.

Please for the next submission do a proper one with a commit message
and a signed-of-by line. This way, we can integrate it when the code
looks clearer or someone else can take over the job if you decide not
to port it upstream.

>
> BR
> G.Baroncelli
>
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index a93cefe..4d907d6 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -35,6 +35,7 @@ MODULE_AUTHOR("Nestor Lopez Casado <nlopezcasad@logitech.com>");
>  #define HIDPP_REPORT_LONG_LENGTH               20
>
>  #define HIDPP_QUIRK_CLASS_WTP                  BIT(0)
> +#define HIDPP_QUIRK_CLASS_M560                 BIT(1)
>
>  /* bits 1..20 are reserved for classes */

Technically speaking, this should be changed to "2..20" :)

>  #define HIDPP_QUIRK_DELAYED_INIT               BIT(21)
> @@ -925,6 +926,225 @@ static void wtp_connect(struct hid_device *hdev, bool connected)
>  }
>
>  /* -------------------------------------------------------------------------- */
> +/* Logitech M560 devices                                                     */
> +/* -------------------------------------------------------------------------- */
> +
> +/*
> + * Logitech M560 protocol overview
> + *
> + * The Logitech M560 mouse, is designed for windows 8. When the middle and/or
> + * the sides buttons are pressed, it sends some keyboard keys events
> + * instead of buttons ones.
> + * To complicate further the things, the middle button keys sequence
> + * is different from the odd press and the even press.
> + *
> + * forward button -> Super_R
> + * backward button -> Super_L+'d' (press only)
> + * middle button -> 1st time: Alt_L+SuperL+XF86TouchpadOff (press only)
> + *                  2nd time: left-click (press only)
> + * NB: press-only means that when the button is pressed, the
> + * KeyPress/ButtonPress and KeyRelease/ButtonRelease events are generated
> + * together sequentially; instead when the button is released, no event is
> + * generated !
> + *
> + * With the command
> + *     10<xx>0a 3500af03 (where <xx> is the mouse id),
> + * the mouse reacts differently:
> + * - it never send a keyboard key event
> + * - for the three mouse button it sends:
> + *     middle button               press   11<xx>0a 3500af00...
> + *     side 1 button (forward)     press   11<xx>0a 3500b000...
> + *     side 2 button (backward)    press   11<xx>0a 3500ae00...
> + *     middle/side1/side2 button   release 11<xx>0a 35000000...
> + */
> +static u8 m560_config_command[] = {0x35, 0x00, 0xaf, 0x03};

please mark this one as const.

> +
> +struct m560_private_data {
> +       u8 prev_data[30]; // FIXME: select a right size

Don't insert FIXME, fix it before the integration :)

> +       int btn_middle:1;
> +       int btn_forward:1;
> +       int btn_backward:1;

using a int per button seems a lot of lost space. Maybe you can use a
bitmask or at least bool (which remaps to an unsigned char IIRC).

> +};
> +
> +/* how the button are mapped in the report */
> +#define MOUSE_BTN_LEFT         0
> +#define MOUSE_BTN_RIGHT                1
> +#define MOUSE_BTN_MIDDLE       2
> +#define MOUSE_BTN_WHEEL_LEFT   3
> +#define MOUSE_BTN_WHEEL_RIGHT  4
> +#define MOUSE_BTN_FORWARD      5
> +#define MOUSE_BTN_BACKWARD     6

Please prefix those defines by M560.

> +
> +/*
> + * m560_priv_data - helper to convert from hidpp_device to m560_private_data
> + *
> + * @hdev: hid device
> + *
> + * @return: return m560_private_data if available, NULL otherwise
> + */
> +static inline struct m560_private_data *m560_priv_data(struct hid_device *hdev)
> +{
> +       struct hidpp_device *hidpp_dev = hid_get_drvdata(hdev);
> +       return hidpp_dev ? hidpp_dev->private_data : NULL;
> +}

Are you sure we really need this check for M560 mice? if you are
calling this in the M560 protocol handling only and made sure in the
probe that private_data gets allocated, no need to double check
things.

> +
> +/*
> + * m560_send_config_command - send the config_command to the mouse
> + *
> + * @dev: hid device where the mouse belongs
> + *
> + * @return: 0 OK
> + */
> +static int m560_send_config_command(struct hid_device *hdev) {
> +       struct hidpp_report response;
> +       struct hidpp_device *hidpp_dev = hid_get_drvdata(hdev);
> +       int ret;
> +
> +       ret = hidpp_send_rap_command_sync(
> +               hidpp_dev,
> +               REPORT_ID_HIDPP_SHORT,
> +               0x0a,

Please use a define for this sub id.
BTW, are you sure the mouse is HID++ 1.0? If it is 2.0, you have to
use the fap protocol, not the rap.

> +               m560_config_command[0],
> +               m560_config_command+1,
> +               sizeof(m560_config_command)-1,

I would personally define m560_config_command[0] in a separate define
(M560_BUTTON_MODE_REGISTER for instance) and keep the parameters to
the right size right now.

> +               &response
> +       );
> +
> +       return ret;
> +}
> +
> +static int m560_allocate(struct hid_device *hdev)
> +{
> +       struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> +       struct m560_private_data *d;
> +
> +       d = devm_kzalloc(&hdev->dev, sizeof(struct m560_private_data),
> +                       GFP_KERNEL);
> +       if (!d)
> +               return -ENOMEM;
> +
> +       hidpp->private_data = d;
> +       //d->hidpp_dev = hidpp;
> +       //d->hid_dev = hdev;

No leftover debug comments please.

> +
> +       return 0;
> +};
> +
> +static inline void set_btn_bit(u8 *data, int bit)
> +{
> +       int bytenr = bit / 8;
> +       int bitmask = 1 << (bit & 0x07);
> +
> +       data[bytenr] |= bitmask;

You seem to be using {set|get|clear}_btn_bit only with MOUSE_BTN_*
which max is 6. You don't really need this helpers and just use plain
bit operations.
If you define MOUSE_BTN_* as BIT(0)..BIT(6), you don't need extra layer at all.

> +}
> +
> +static inline int get_btn_bit(u8 *data, int bit)
> +{
> +       int bytenr = bit / 8;
> +       int bitmask = 1 << (bit & 0x07);
> +
> +       return !!(data[bytenr] & bitmask);
> +}
> +
> +static inline void clear_btn_bit(u8 *data, int bit)
> +{
> +       int bytenr = bit / 8;
> +       int bitmask = 1 << (bit & 0x07);
> +
> +       data[bytenr] &= ~bitmask;
> +}
> +
> +static int m560_raw_event(struct hid_device *hdev, u8 *data, int size)
> +{
> +       struct m560_private_data *mydata = m560_priv_data(hdev);
> +
> +       /* check if the data is a mouse related report */
> +       if (data[0] != 0x02 && data[2] != 0x0a)

Please no magical numbers. You should have these defined somewhere
(OK, I did not define 0x02 myself, but you should!)

> +               return 1;
> +
> +       /* check if the report is the ack of the config_command */
> +       if (data[0] == 0x11 && data[2] == 0x0a &&

0x11 is REPORT_ID_HIDPP_LONG

> +           size >= (3+sizeof(m560_config_command)) &&
> +           !memcmp(data+3, m560_config_command,
> +               sizeof(m560_config_command))) {
> +                       return true;

this whole test is a lot messy. There should be a way to clean it a
little (maybe just define what you expect, and use one memcmp).

> +       }
> +
> +       if (data[0] == 0x11 && data[2] == 0x0a && data[06] == 0x00) {
> +               /*
> +                * m560 mouse button report
> +                *
> +                * data[0] = 0x11
> +                * data[1] = deviceid
> +                * data[2] = 0x0a
> +                * data[5] = button (0xaf->middle, 0xb0->forward,
> +                *                   0xaf ->backward, 0x00->release all)
> +                * data[6] = 0x00
> +                */
> +
> +               int btn, i, maxsize;
> +
> +               /* check if the event is a button */
> +               btn = data[5];
> +               if (btn != 0x00 && btn != 0xb0 && btn != 0xae && btn != 0xaf)
> +                       return true;

I wouldn't be surprised if these buttons where just bitmasks. Do you
still get the same values if you press 2/3 buttons at the same time?

0xa0 seems common to all these bytes (0b10100000).
BIT(0) seems to be btn_middle
BIT(4) seems to be btn_forward

but btn_backward does not make any sense.
Nestor, can you tell us what these magic packets are?

> +
> +               if (btn == 0xaf)
> +                       mydata->btn_middle = 1;
> +               else if (btn == 0xb0)
> +                       mydata->btn_forward = 1;
> +               else if (btn == 0xae)
> +                       mydata->btn_backward = 1;
> +               else if (btn == 0x00) {
> +                       mydata->btn_backward = 0;
> +                       mydata->btn_forward = 0;
> +                       mydata->btn_middle = 0;
> +               }

As mentioned above, this looks fragile if 2 buttons are pressed at the
same time, the first one will be spuriously released.

> +
> +               /* replace the report with the old one */
> +               if (size > sizeof(mydata->prev_data))
> +                       maxsize = sizeof(mydata->prev_data);
> +               else
> +                       maxsize = size;
> +               for (i = 0 ; i < maxsize ; i++)
> +                       data[i] = mydata->prev_data[i];

I think here and later, you are making things too complicated. Can't
you just emit the buttons when retrieving them with input_event(), and
call input_sync() now and drop the whole rewriting of HID report which
just add some overhead and complexity.

> +
> +       } else if (data[0] == 0x02) {
> +               /*
> +                * standard mouse report
> +                *
> +                * data[0] = type (0x02)
> +                * data[1..2] = buttons
> +                * data[3..5] = xy
> +                * data[6] = wheel
> +                * data[7] = horizontal wheel
> +                */
> +
> +               /* horizontal wheel handling */
> +               if (get_btn_bit(data+1,MOUSE_BTN_WHEEL_LEFT))
> +                       data[1+6] = -1;
> +               if (get_btn_bit(data+1,MOUSE_BTN_WHEEL_RIGHT))
> +                       data[1+6] =  1;

Do we really need to clear those buttons? Xorg should be able to remap
them to BTN_HORIZ_WHEEL or so.

> +
> +               clear_btn_bit(data+1, MOUSE_BTN_WHEEL_LEFT);
> +               clear_btn_bit(data+1, MOUSE_BTN_WHEEL_RIGHT);
> +
> +               /* copy the type and buttons status */
> +               memcpy(mydata->prev_data, data, 3);

This can be dropped if you directly call input_event in the previous case.

> +       }
> +
> +       /* add the extra buttons */
> +       if (mydata->btn_middle)
> +               set_btn_bit(data+1, MOUSE_BTN_MIDDLE);
> +       if (mydata->btn_forward)
> +               set_btn_bit(data+1, MOUSE_BTN_FORWARD);
> +       if (mydata->btn_backward)
> +               set_btn_bit(data+1, MOUSE_BTN_BACKWARD);

This is somewhat disturbing. I think it is fine, but an other solution
would be to simply ignore the mapping of those buttons in
input_mapping.

> +
> +       return 1;
> +}
> +
> +/* -------------------------------------------------------------------------- */
>  /* Generic HID++ devices                                                      */
>  /* -------------------------------------------------------------------------- */
>
> @@ -936,6 +1156,9 @@ static int hidpp_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>
>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>                 return wtp_input_mapping(hdev, hi, field, usage, bit, max);
> +       if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560 &&
> +               field->application != HID_GD_MOUSE)
> +                               return -1;
>
>         return 0;
>  }
> @@ -998,6 +1221,8 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
>
>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>                 return wtp_raw_event(hidpp->hid_dev, data, size);
> +       if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
> +               return m560_raw_event(hidpp->hid_dev, data, size);
>
>         return 0;
>  }
> @@ -1026,6 +1251,8 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
>
>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>                 return wtp_raw_event(hdev, data, size);
> +       if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
> +               return m560_raw_event(hidpp->hid_dev, data, size);
>
>         return 0;
>  }
> @@ -1100,6 +1327,8 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
>
>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>                 wtp_connect(hdev, connected);
> +       if ((hidpp->quirks & HIDPP_QUIRK_CLASS_M560) && connected)
> +               m560_send_config_command(hdev);
>
>         if (!connected || hidpp->delayed_input)
>                 return;
> @@ -1164,6 +1393,11 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>                 if (ret)
>                         goto wtp_allocate_fail;
>         }
> +       if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) {
> +               ret = m560_allocate(hdev);
> +               if (ret)
> +                       goto wtp_allocate_fail;

This looks weird. we should probably rename the label to
"class_allocate_fail" or something similar.

> +       }
>
>         INIT_WORK(&hidpp->work, delayed_work_cb);
>         mutex_init(&hidpp->send_mutex);
> @@ -1240,6 +1474,7 @@ static void hidpp_remove(struct hid_device *hdev)
>         cancel_work_sync(&hidpp->work);
>         mutex_destroy(&hidpp->send_mutex);
>         hid_hw_stop(hdev);
> +

Please no extra line out of context.

>  }
>
>  static const struct hid_device_id hidpp_devices[] = {
> @@ -1261,6 +1496,12 @@ static const struct hid_device_id hidpp_devices[] = {
>                 USB_VENDOR_ID_LOGITECH, 0x4102),
>           .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_MULTI_INPUT |
>                          HIDPP_QUIRK_CLASS_WTP },
> +       { /* Mouse logitech M560 */
> +         HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
> +               USB_VENDOR_ID_LOGITECH, 0x402d),
> +         .driver_data = HIDPP_QUIRK_CLASS_M560 | HIDPP_QUIRK_DELAYED_INIT |
> +                        HIDPP_QUIRK_MULTI_INPUT
> +       },
>
>         { HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
>                 USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
>
>

One last comment. Please check the patch against
./script/checkpatch.pl. There are here and ther some whitespace
problems that checkpatch will raise.


Thanks for the submission.

Cheers,
Benjamin

>
> On 2015-04-09 19:35, Benjamin Tissoires wrote:
>> Hi Antonio,
>>
>> On Thu, Apr 9, 2015 at 8:41 AM, Antonio Ospite <ao2@ao2.it> wrote:
>>> On Fri, 05 Sep 2014 19:47:44 +0200
>>> Goffredo Baroncelli <kreijack@inwind.it> wrote:
>>>
>>>> On 09/03/2014 11:36 PM, Benjamin Tissoires wrote:
>>>>> Hi Goffredo,
>>>>>
>>>>> On Mon, Sep 1, 2014 at 3:20 PM, Goffredo Baroncelli <kreijack@inwind.it> wrote:
>>>>>> Hi Benjamin,
>>>>>>
>>>>>> following the Nestor suggestion, I rewrote the driver for the
>>>>>> mouse Logitech M560 on the basis of your work (branch "for-whot").
>>>>>
>>>>> Just for the record. This branch is located here:
>>>>> https://github.com/bentiss/hid-logitech-dj/tree/for-whot and I
>>>>> *really* need to finish this work so that everything can go upstream
>>>>> :(
>>>>
>>>
>>> Hi Benjamin, any news about upstreaming this work?
>>
>> The hidpp / raw touchpad mode is already upstream since the v3.19 kernel. \o/
>>
>> We just need to port Goffredo's changes now and push them too.
>> There has been some differences since this branch was published. The
>> main is that there is no more special subdrivers in several files,
>> everything is handled in hidpp. I think the logic behind is somewhat
>> the same so it should not be too much a problem to do the work.
>>
>> Cheers,
>> Benjamin
>>
>>>
>>>> :-) For me this is not a problem. I solved my issue (I made a dkms
>>>> package for this mouse), but I want to share my effort..
>>>>
>>>
>>> Goffredo, is this the latest version of your dkms-enabled external
>>> module?
>>> https://github.com/kreijack/hid-logitech-dj/tree/m560-dkms
>>>
>>> Thanks,
>>>    Antonio
>>>
>>> --
>>> Antonio Ospite
>>> http://ao2.it
>>>
>>> A: Because it messes up the order in which people normally read text.
>>>    See http://en.wikipedia.org/wiki/Posting_style
>>> Q: Why is top-posting such a bad thing?
>>
>
>
> --
> gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
> Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: Driver for Logitech M560

[Goffredo Baroncelli]

On 2015-04-13 17:06, Benjamin Tissoires wrote:
> On Fri, Apr 10, 2015 at 2:56 PM, Goffredo Baroncelli <kreijack@inwind.it> wrote:
>> Hi,
>>
>> after the Antonio's mail, I updated my work on the latest kernel (3.19.3); this work for me, but I am sure that I made some mistakes, so please consider this a beta.
> 
> Thanks for the patch. I have some comments, please see inline.
> 
>> I added a new device class (M560), and I put some hooks to process the raw data.
>>
>> I used the following "quirks":
>> - HIDPP_QUIRK_DELAYED_INIT
>> - HIDPP_QUIRK_MULTI_INPUT
> 
> I don't think you really need HIDPP_QUIRK_MULTI_INPUT. If the mouse
> has only one input node (or you want to send only on the input node),
> then you don't need to keep the keyboard node.
> 
> [10 min later]
> 
> OK, after reviewing the code, I see why you kept
> HIDPP_QUIRK_MULTI_INPUT. I am not entirely sure what is the best
> solution: keeping it and rely on hid-input for the parsing/allocating
> of regular mouse events and having .raw_event() tampering with the raw
> report, or just trash hid-input and process entirely the incoming
> report...



> 
>>
>> these work (when the mouse is connected, the configuration message is sent), but I am not sure if this is correct.
>> Anyway in the past I noticed some false spurious reconnections, and re-sending the configuration time to time made the mouse not-responsive for few tens of second (it seems a low value, but I noticed it); so I am not sure if I will leave the "reconfiguration" on reconnection.
>>
>> Comments are welcome.
> 
> Please for the next submission do a proper one with a commit message
> and a signed-of-by line. This way, we can integrate it when the code
> looks clearer or someone else can take over the job if you decide not
> to port it upstream.
> 
>>
>> BR
>> G.Baroncelli
>>
>>
>> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
>> index a93cefe..4d907d6 100644
>> --- a/drivers/hid/hid-logitech-hidpp.c
>> +++ b/drivers/hid/hid-logitech-hidpp.c
>> @@ -35,6 +35,7 @@ MODULE_AUTHOR("Nestor Lopez Casado <nlopezcasad@logitech.com>");
>>  #define HIDPP_REPORT_LONG_LENGTH               20
>>
>>  #define HIDPP_QUIRK_CLASS_WTP                  BIT(0)
>> +#define HIDPP_QUIRK_CLASS_M560                 BIT(1)
>>
>>  /* bits 1..20 are reserved for classes */
> 
> Technically speaking, this should be changed to "2..20" :)
ok
> 
>>  #define HIDPP_QUIRK_DELAYED_INIT               BIT(21)
>> @@ -925,6 +926,225 @@ static void wtp_connect(struct hid_device *hdev, bool connected)
>>  }
>>
>>  /* -------------------------------------------------------------------------- */
>> +/* Logitech M560 devices                                                     */
>> +/* -------------------------------------------------------------------------- */
>> +
>> +/*
>> + * Logitech M560 protocol overview
>> + *
>> + * The Logitech M560 mouse, is designed for windows 8. When the middle and/or
>> + * the sides buttons are pressed, it sends some keyboard keys events
>> + * instead of buttons ones.
>> + * To complicate further the things, the middle button keys sequence
>> + * is different from the odd press and the even press.
>> + *
>> + * forward button -> Super_R
>> + * backward button -> Super_L+'d' (press only)
>> + * middle button -> 1st time: Alt_L+SuperL+XF86TouchpadOff (press only)
>> + *                  2nd time: left-click (press only)
>> + * NB: press-only means that when the button is pressed, the
>> + * KeyPress/ButtonPress and KeyRelease/ButtonRelease events are generated
>> + * together sequentially; instead when the button is released, no event is
>> + * generated !
>> + *
>> + * With the command
>> + *     10<xx>0a 3500af03 (where <xx> is the mouse id),
>> + * the mouse reacts differently:
>> + * - it never send a keyboard key event
>> + * - for the three mouse button it sends:
>> + *     middle button               press   11<xx>0a 3500af00...
>> + *     side 1 button (forward)     press   11<xx>0a 3500b000...
>> + *     side 2 button (backward)    press   11<xx>0a 3500ae00...
>> + *     middle/side1/side2 button   release 11<xx>0a 35000000...
>> + */
>> +static u8 m560_config_command[] = {0x35, 0x00, 0xaf, 0x03};
> 
> please mark this one as const.
> 
>> +
>> +struct m560_private_data {
>> +       u8 prev_data[30]; // FIXME: select a right size
> 
> Don't insert FIXME, fix it before the integration :)
ok
> 
>> +       int btn_middle:1;
>> +       int btn_forward:1;
>> +       int btn_backward:1;
> 
> using a int per button seems a lot of lost space. Maybe you can use a
> bitmask or at least bool (which remaps to an unsigned char IIRC).
ok

> 
>> +};
>> +
>> +/* how the button are mapped in the report */
>> +#define MOUSE_BTN_LEFT         0
>> +#define MOUSE_BTN_RIGHT                1
>> +#define MOUSE_BTN_MIDDLE       2
>> +#define MOUSE_BTN_WHEEL_LEFT   3
>> +#define MOUSE_BTN_WHEEL_RIGHT  4
>> +#define MOUSE_BTN_FORWARD      5
>> +#define MOUSE_BTN_BACKWARD     6
> 
> Please prefix those defines by M560.

ok

>> +
>> +/*
>> + * m560_priv_data - helper to convert from hidpp_device to m560_private_data
>> + *
>> + * @hdev: hid device
>> + *
>> + * @return: return m560_private_data if available, NULL otherwise
>> + */
>> +static inline struct m560_private_data *m560_priv_data(struct hid_device *hdev)
>> +{
>> +       struct hidpp_device *hidpp_dev = hid_get_drvdata(hdev);
>> +       return hidpp_dev ? hidpp_dev->private_data : NULL;
>> +}
> 
> Are you sure we really need this check for M560 mice? if you are
> calling this in the M560 protocol handling only and made sure in the
> probe that private_data gets allocated, no need to double check
> things.

I will remove, because there was used only one time

>> +
>> +/*
>> + * m560_send_config_command - send the config_command to the mouse
>> + *
>> + * @dev: hid device where the mouse belongs
>> + *
>> + * @return: 0 OK
>> + */
>> +static int m560_send_config_command(struct hid_device *hdev) {
>> +       struct hidpp_report response;
>> +       struct hidpp_device *hidpp_dev = hid_get_drvdata(hdev);
>> +       int ret;
>> +
>> +       ret = hidpp_send_rap_command_sync(
>> +               hidpp_dev,
>> +               REPORT_ID_HIDPP_SHORT,
>> +               0x0a,
> 
> Please use a define for this sub id.
> BTW, are you sure the mouse is HID++ 1.0? If it is 2.0, you have to
> use the fap protocol, not the rap.

I don't know :-) ***************

> 
>> +               m560_config_command[0],
>> +               m560_config_command+1,
>> +               sizeof(m560_config_command)-1,
> 
> I would personally define m560_config_command[0] in a separate define
> (M560_BUTTON_MODE_REGISTER for instance) and keep the parameters to
> the right size right now.

I rearranged a bit the code
 
>> +               &response
>> +       );
>> +
>> +       return ret;
>> +}
>> +
>> +static int m560_allocate(struct hid_device *hdev)
>> +{
>> +       struct hidpp_device *hidpp = hid_get_drvdata(hdev);
>> +       struct m560_private_data *d;
>> +
>> +       d = devm_kzalloc(&hdev->dev, sizeof(struct m560_private_data),
>> +                       GFP_KERNEL);
>> +       if (!d)
>> +               return -ENOMEM;
>> +
>> +       hidpp->private_data = d;
>> +       //d->hidpp_dev = hidpp;
>> +       //d->hid_dev = hdev;
> 
> No leftover debug comments please.

ok
 
>> +
>> +       return 0;
>> +};
>> +
>> +static inline void set_btn_bit(u8 *data, int bit)
>> +{
>> +       int bytenr = bit / 8;
>> +       int bitmask = 1 << (bit & 0x07);
>> +
>> +       data[bytenr] |= bitmask;
> 
> You seem to be using {set|get|clear}_btn_bit only with MOUSE_BTN_*
> which max is 6. You don't really need this helpers and just use plain
> bit operations.
> If you define MOUSE_BTN_* as BIT(0)..BIT(6), you don't need extra layer at all.
ok
> 
>> +}
>> +
>> +static inline int get_btn_bit(u8 *data, int bit)
>> +{
>> +       int bytenr = bit / 8;
>> +       int bitmask = 1 << (bit & 0x07);
>> +
>> +       return !!(data[bytenr] & bitmask);
>> +}
>> +
>> +static inline void clear_btn_bit(u8 *data, int bit)
>> +{
>> +       int bytenr = bit / 8;
>> +       int bitmask = 1 << (bit & 0x07);
>> +
>> +       data[bytenr] &= ~bitmask;
>> +}
>> +
>> +static int m560_raw_event(struct hid_device *hdev, u8 *data, int size)
>> +{
>> +       struct m560_private_data *mydata = m560_priv_data(hdev);
>> +
>> +       /* check if the data is a mouse related report */
>> +       if (data[0] != 0x02 && data[2] != 0x0a)
> 
> Please no magical numbers. You should have these defined somewhere
> (OK, I did not define 0x02 myself, but you should!)
> 
>> +               return 1;
>> +
>> +       /* check if the report is the ack of the config_command */
>> +       if (data[0] == 0x11 && data[2] == 0x0a &&
> 
> 0x11 is REPORT_ID_HIDPP_LONG
ok

> 
>> +           size >= (3+sizeof(m560_config_command)) &&
>> +           !memcmp(data+3, m560_config_command,
>> +               sizeof(m560_config_command))) {
>> +                       return true;
> 
> this whole test is a lot messy. There should be a way to clean it a
> little (maybe just define what you expect, and use one memcmp).

I removed this check, because the configure command is send in another way


>> +       }
>> +
>> +       if (data[0] == 0x11 && data[2] == 0x0a && data[06] == 0x00) {
>> +               /*
>> +                * m560 mouse button report
>> +                *
>> +                * data[0] = 0x11
>> +                * data[1] = deviceid
>> +                * data[2] = 0x0a
>> +                * data[5] = button (0xaf->middle, 0xb0->forward,
>> +                *                   0xaf ->backward, 0x00->release all)
>> +                * data[6] = 0x00
>> +                */
>> +
>> +               int btn, i, maxsize;
>> +
>> +               /* check if the event is a button */
>> +               btn = data[5];
>> +               if (btn != 0x00 && btn != 0xb0 && btn != 0xae && btn != 0xaf)
>> +                       return true;
> 
> I wouldn't be surprised if these buttons where just bitmasks. Do you
> still get the same values if you press 2/3 buttons at the same time?
> 
> 0xa0 seems common to all these bytes (0b10100000).
> BIT(0) seems to be btn_middle
> BIT(4) seems to be btn_forward
> 
> but btn_backward does not make any sense.
> Nestor, can you tell us what these magic packets are?
> 
>> +
>> +               if (btn == 0xaf)
>> +                       mydata->btn_middle = 1;
>> +               else if (btn == 0xb0)
>> +                       mydata->btn_forward = 1;
>> +               else if (btn == 0xae)
>> +                       mydata->btn_backward = 1;
>> +               else if (btn == 0x00) {
>> +                       mydata->btn_backward = 0;
>> +                       mydata->btn_forward = 0;
>> +                       mydata->btn_middle = 0;
>> +               }
> 
> As mentioned above, this looks fragile if 2 buttons are pressed at the
> same time, the first one will be spuriously released.

Unfortunately, when the two button are released the same bytes sequence is emitted :-(
There is no way to distinguish the three release events

> 
>> +
>> +               /* replace the report with the old one */
>> +               if (size > sizeof(mydata->prev_data))
>> +                       maxsize = sizeof(mydata->prev_data);
>> +               else
>> +                       maxsize = size;
>> +               for (i = 0 ; i < maxsize ; i++)
>> +                       data[i] = mydata->prev_data[i];
> 
> I think here and later, you are making things too complicated. Can't
> you just emit the buttons when retrieving them with input_event(), and
> call input_sync() now and drop the whole rewriting of HID report which
> just add some overhead and complexity.

I need more input, because I am not familiar with input_event()/input_sync(): I tried
to replace the changing of the report with something like:

	if (btn == 0xaf) {
		input_event(mydata->input, EV_KEY, BTN_MIDDLE, 1);
		input_sync(mydata->input);
		return 0;
	}

But the result was that I never got a middle-button event... I missing something.

> 
>> +
>> +       } else if (data[0] == 0x02) {
>> +               /*
>> +                * standard mouse report
>> +                *
>> +                * data[0] = type (0x02)
>> +                * data[1..2] = buttons
>> +                * data[3..5] = xy
>> +                * data[6] = wheel
>> +                * data[7] = horizontal wheel
>> +                */
>> +
>> +               /* horizontal wheel handling */
>> +               if (get_btn_bit(data+1,MOUSE_BTN_WHEEL_LEFT))
>> +                       data[1+6] = -1;
>> +               if (get_btn_bit(data+1,MOUSE_BTN_WHEEL_RIGHT))
>> +                       data[1+6] =  1;
> 
> Do we really need to clear those buttons? Xorg should be able to remap
> them to BTN_HORIZ_WHEEL or so.

Maybe, but so it works out of the box without xmodmap ...

> 
>> +
>> +               clear_btn_bit(data+1, MOUSE_BTN_WHEEL_LEFT);
>> +               clear_btn_bit(data+1, MOUSE_BTN_WHEEL_RIGHT);
>> +
>> +               /* copy the type and buttons status */
>> +               memcpy(mydata->prev_data, data, 3);
> 
> This can be dropped if you directly call input_event in the previous case.
> 
>> +       }
>> +
>> +       /* add the extra buttons */
>> +       if (mydata->btn_middle)
>> +               set_btn_bit(data+1, MOUSE_BTN_MIDDLE);
>> +       if (mydata->btn_forward)
>> +               set_btn_bit(data+1, MOUSE_BTN_FORWARD);
>> +       if (mydata->btn_backward)
>> +               set_btn_bit(data+1, MOUSE_BTN_BACKWARD);
> 
> This is somewhat disturbing. I think it is fine, but an other solution
> would be to simply ignore the mapping of those buttons in
> input_mapping.
> 
>> +
>> +       return 1;
>> +}
>> +
>> +/* -------------------------------------------------------------------------- */
>>  /* Generic HID++ devices                                                      */
>>  /* -------------------------------------------------------------------------- */
>>
>> @@ -936,6 +1156,9 @@ static int hidpp_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>>
>>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>>                 return wtp_input_mapping(hdev, hi, field, usage, bit, max);
>> +       if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560 &&
>> +               field->application != HID_GD_MOUSE)
>> +                               return -1;
>>
>>         return 0;
>>  }
>> @@ -998,6 +1221,8 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
>>
>>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>>                 return wtp_raw_event(hidpp->hid_dev, data, size);
>> +       if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
>> +               return m560_raw_event(hidpp->hid_dev, data, size);
>>
>>         return 0;
>>  }
>> @@ -1026,6 +1251,8 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
>>
>>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>>                 return wtp_raw_event(hdev, data, size);
>> +       if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
>> +               return m560_raw_event(hidpp->hid_dev, data, size);
>>
>>         return 0;
>>  }
>> @@ -1100,6 +1327,8 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
>>
>>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>>                 wtp_connect(hdev, connected);
>> +       if ((hidpp->quirks & HIDPP_QUIRK_CLASS_M560) && connected)
>> +               m560_send_config_command(hdev);
>>
>>         if (!connected || hidpp->delayed_input)
>>                 return;
>> @@ -1164,6 +1393,11 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>                 if (ret)
>>                         goto wtp_allocate_fail;
>>         }
>> +       if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) {
>> +               ret = m560_allocate(hdev);
>> +               if (ret)
>> +                       goto wtp_allocate_fail;
> 
> This looks weird. we should probably rename the label to
> "class_allocate_fail" or something similar.
ok
> 
>> +       }
>>
>>         INIT_WORK(&hidpp->work, delayed_work_cb);
>>         mutex_init(&hidpp->send_mutex);
>> @@ -1240,6 +1474,7 @@ static void hidpp_remove(struct hid_device *hdev)
>>         cancel_work_sync(&hidpp->work);
>>         mutex_destroy(&hidpp->send_mutex);
>>         hid_hw_stop(hdev);
>> +
> 
> Please no extra line out of context.
ok


>>  }
>>
>>  static const struct hid_device_id hidpp_devices[] = {
>> @@ -1261,6 +1496,12 @@ static const struct hid_device_id hidpp_devices[] = {
>>                 USB_VENDOR_ID_LOGITECH, 0x4102),
>>           .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_MULTI_INPUT |
>>                          HIDPP_QUIRK_CLASS_WTP },
>> +       { /* Mouse logitech M560 */
>> +         HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
>> +               USB_VENDOR_ID_LOGITECH, 0x402d),
>> +         .driver_data = HIDPP_QUIRK_CLASS_M560 | HIDPP_QUIRK_DELAYED_INIT |
>> +                        HIDPP_QUIRK_MULTI_INPUT
>> +       },
>>
>>         { HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
>>                 USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
>>
>>
> 
> One last comment. Please check the patch against
> ./script/checkpatch.pl. There are here and ther some whitespace
> problems that checkpatch will raise.
Ok


> Thanks for the submission.

> 
> Cheers,
> Benjamin
> 
>>
>> On 2015-04-09 19:35, Benjamin Tissoires wrote:
>>> Hi Antonio,
>>>
>>> On Thu, Apr 9, 2015 at 8:41 AM, Antonio Ospite <ao2@ao2.it> wrote:
>>>> On Fri, 05 Sep 2014 19:47:44 +0200
>>>> Goffredo Baroncelli <kreijack@inwind.it> wrote:
>>>>
>>>>> On 09/03/2014 11:36 PM, Benjamin Tissoires wrote:
>>>>>> Hi Goffredo,
>>>>>>
>>>>>> On Mon, Sep 1, 2014 at 3:20 PM, Goffredo Baroncelli <kreijack@inwind.it> wrote:
>>>>>>> Hi Benjamin,
>>>>>>>
>>>>>>> following the Nestor suggestion, I rewrote the driver for the
>>>>>>> mouse Logitech M560 on the basis of your work (branch "for-whot").
>>>>>>
>>>>>> Just for the record. This branch is located here:
>>>>>> https://github.com/bentiss/hid-logitech-dj/tree/for-whot and I
>>>>>> *really* need to finish this work so that everything can go upstream
>>>>>> :(
>>>>>
>>>>
>>>> Hi Benjamin, any news about upstreaming this work?
>>>
>>> The hidpp / raw touchpad mode is already upstream since the v3.19 kernel. \o/
>>>
>>> We just need to port Goffredo's changes now and push them too.
>>> There has been some differences since this branch was published. The
>>> main is that there is no more special subdrivers in several files,
>>> everything is handled in hidpp. I think the logic behind is somewhat
>>> the same so it should not be too much a problem to do the work.
>>>
>>> Cheers,
>>> Benjamin
>>>
>>>>
>>>>> :-) For me this is not a problem. I solved my issue (I made a dkms
>>>>> package for this mouse), but I want to share my effort..
>>>>>
>>>>
>>>> Goffredo, is this the latest version of your dkms-enabled external
>>>> module?
>>>> https://github.com/kreijack/hid-logitech-dj/tree/m560-dkms
>>>>
>>>> Thanks,
>>>>    Antonio
>>>>
>>>> --
>>>> Antonio Ospite
>>>> http://ao2.it
>>>>
>>>> A: Because it messes up the order in which people normally read text.
>>>>    See http://en.wikipedia.org/wiki/Posting_style
>>>> Q: Why is top-posting such a bad thing?
>>>
>>
>>
>> --
>> gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
>> Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

[Patch V2] Driver for Logitech M560

[Goffredo Baroncelli]

Hi Benjamin,


I accepted near all your suggestions; the one which I leaved out is the
one related to using input_event()/input_sync() instead of rewriting
the hid report. I am not against to it, but... I don't know enough the hid
stack, so I need some more suggestions from your side.

In the meantime, I updated the patch on the basis of your suggestions.



BR
G.Baroncelli

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

----

The Logitech M560 is is a mouse designed for windows 8. 
Comparing to a standard one, some buttons (the middle one and the 
two ones placed on the side) are bounded to a key combination 
instead of a classic "mouse" button.

Think this mouse as a pair of mouse and keyboard. When the middle
button is pressed the it sends a key (as keyboard) 
combination, the same for the other two side button.
Instead the left/right/wheel work correctly.
To complicate further the things, the middle button send a
key combination the odd press, and another one for the even press;
in the latter case it sends also a left click. But the worst thing 
is that no event is generated when the middle button is released.

Moreover this device is a wireless mouse which uses the unifying 
receiver.

I discovered that it is possible to re-configure the mouse
sending a command (see function m560_send_config_command()).
After this command the mouse sends some sequence when the
buttons are pressed and/or released (see comments for
an explanation of the mouse protocol).

This patch update the file driver/hid/hid-logitech-hidpp.c (v3.19.3)

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>

Changelog:
- v1 first issue
- v2 accepted Benjamin Tissoires suggestions

diff --git a/hid-logitech-hidpp.c b/hid-logitech-hidpp.c
index a93cefe..805ecb6 100644
--- a/hid-logitech-hidpp.c
+++ b/hid-logitech-hidpp.c
@@ -35,8 +35,9 @@ MODULE_AUTHOR("Nestor Lopez Casado <nlopezcasad@logitech.com>");
 #define HIDPP_REPORT_LONG_LENGTH		20
 
 #define HIDPP_QUIRK_CLASS_WTP			BIT(0)
+#define HIDPP_QUIRK_CLASS_M560			BIT(1)
 
-/* bits 1..20 are reserved for classes */
+/* bits 2..20 are reserved for classes */
 #define HIDPP_QUIRK_DELAYED_INIT		BIT(21)
 #define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS	BIT(22)
 #define HIDPP_QUIRK_MULTI_INPUT			BIT(23)
@@ -925,6 +926,177 @@ static void wtp_connect(struct hid_device *hdev, bool connected)
 }
 
 /* -------------------------------------------------------------------------- */
+/* Logitech M560 devices                                                     */
+/* -------------------------------------------------------------------------- */
+
+/*
+ * Logitech M560 protocol overview
+ *
+ * The Logitech M560 mouse, is designed for windows 8. When the middle and/or
+ * the sides buttons are pressed, it sends some keyboard keys events
+ * instead of buttons ones.
+ * To complicate further the things, the middle button keys sequence
+ * is different from the odd press and the even press.
+ *
+ * forward button -> Super_R
+ * backward button -> Super_L+'d' (press only)
+ * middle button -> 1st time: Alt_L+SuperL+XF86TouchpadOff (press only)
+ *                  2nd time: left-click (press only)
+ * NB: press-only means that when the button is pressed, the
+ * KeyPress/ButtonPress and KeyRelease/ButtonRelease events are generated
+ * together sequentially; instead when the button is released, no event is
+ * generated !
+ *
+ * With the command
+ *	10<xx>0a 3500af03 (where <xx> is the mouse id),
+ * the mouse reacts differently:
+ * - it never send a keyboard key event
+ * - for the three mouse button it sends:
+ *	middle button               press   11<xx>0a 3500af00...
+ *	side 1 button (forward)     press   11<xx>0a 3500b000...
+ *	side 2 button (backward)    press   11<xx>0a 3500ae00...
+ *	middle/side1/side2 button   release 11<xx>0a 35000000...
+ */
+
+static const u8 m560_config_parameter[] = {0x00, 0xaf, 0x03};
+
+struct m560_private_data {
+	u8 prev_data[10];
+	u8 button_pressed;
+};
+
+/* how the button are mapped in the report */
+#define M560_MOUSE_BTN_LEFT		0x01
+#define M560_MOUSE_BTN_RIGHT		0x02
+#define M560_MOUSE_BTN_MIDDLE		0x04
+#define M560_MOUSE_BTN_WHEEL_LEFT	0x08
+#define M560_MOUSE_BTN_WHEEL_RIGHT	0x10
+#define M560_MOUSE_BTN_FORWARD		0x20
+#define M560_MOUSE_BTN_BACKWARD		0x40
+#define M560_SUB_ID			0x0a
+#define M560_BUTTON_MODE_REGISTER	0x35
+
+/*
+ * m560_send_config_command - send the config_command to the mouse
+ *
+ * @dev: hid device where the mouse belongs
+ *
+ * @return: 0 OK
+ */
+static int m560_send_config_command(struct hid_device *hdev)
+{
+	struct hidpp_report response;
+	struct hidpp_device *hidpp_dev = hid_get_drvdata(hdev);
+	int ret;
+
+	ret = hidpp_send_rap_command_sync(
+		hidpp_dev,
+		REPORT_ID_HIDPP_SHORT,
+		M560_SUB_ID,
+		M560_BUTTON_MODE_REGISTER,
+		(u8 *)m560_config_parameter,
+		sizeof(m560_config_parameter),
+		&response
+	);
+
+	return ret;
+}
+
+static int m560_allocate(struct hid_device *hdev)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+	struct m560_private_data *d;
+
+	d = devm_kzalloc(&hdev->dev, sizeof(struct m560_private_data),
+			GFP_KERNEL);
+	if (!d)
+		return -ENOMEM;
+
+	hidpp->private_data = d;
+
+	return 0;
+};
+
+static int m560_raw_event(struct hid_device *hdev, u8 *data, int size)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+	struct m560_private_data *mydata = hidpp->private_data;
+	u8 *btn_ptr = data+1;
+	u8 *wheel_ptr = data+1+6;
+
+	/* check if the data is a mouse related report */
+	if (data[0] != 0x02 && data[2] != M560_SUB_ID)
+		return 1;
+
+	if (data[0] == REPORT_ID_HIDPP_LONG &&
+	    data[2] == M560_SUB_ID && data[06] == 0x00) {
+		/*
+		 * m560 mouse button report
+		 *
+		 * data[0] = 0x11
+		 * data[1] = deviceid
+		 * data[2] = 0x0a
+		 * data[5] = button (0xaf->middle, 0xb0->forward,
+		 *                   0xaf ->backward, 0x00->release all)
+		 * data[6] = 0x00
+		 */
+
+		int btn, i, maxsize;
+
+		/* check if the event is a button */
+		btn = data[5];
+		if (btn != 0x00 && btn != 0xb0 && btn != 0xae && btn != 0xaf)
+			return 1;
+
+		if (btn == 0xaf)
+			mydata->button_pressed |= M560_MOUSE_BTN_MIDDLE;
+		else if (btn == 0xb0)
+			mydata->button_pressed |= M560_MOUSE_BTN_FORWARD;
+		else if (btn == 0xae)
+			mydata->button_pressed |= M560_MOUSE_BTN_BACKWARD;
+		else if (btn == 0x00)
+			mydata->button_pressed &= ~(M560_MOUSE_BTN_BACKWARD|
+				M560_MOUSE_BTN_MIDDLE|M560_MOUSE_BTN_FORWARD);
+
+		/* replace the report with the old one */
+		if (size > sizeof(mydata->prev_data))
+			maxsize = sizeof(mydata->prev_data);
+		else
+			maxsize = size;
+		for (i = 0 ; i < maxsize ; i++)
+			data[i] = mydata->prev_data[i];
+
+	} else if (data[0] == 0x02) {
+		/*
+		 * standard mouse report
+		 *
+		 * data[0] = type (0x02)
+		 * data[1..2] = buttons
+		 * data[3..5] = xy
+		 * data[6] = wheel
+		 * data[7] = horizontal wheel
+		 */
+
+		/* horizontal wheel handling */
+		if (*btn_ptr & M560_MOUSE_BTN_WHEEL_LEFT)
+			*wheel_ptr = -1;
+		if (*btn_ptr & M560_MOUSE_BTN_WHEEL_RIGHT)
+			*wheel_ptr =  1;
+
+		*btn_ptr &= ~(M560_MOUSE_BTN_WHEEL_LEFT|
+				M560_MOUSE_BTN_WHEEL_RIGHT);
+
+		/* copy the type and buttons status */
+		memcpy(mydata->prev_data, data, 3);
+	}
+
+	/* add the extra buttons */
+	*btn_ptr |= mydata->button_pressed;
+
+	return 1;
+}
+
+/* -------------------------------------------------------------------------- */
 /* Generic HID++ devices                                                      */
 /* -------------------------------------------------------------------------- */
 
@@ -936,6 +1108,9 @@ static int hidpp_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		return wtp_input_mapping(hdev, hi, field, usage, bit, max);
+	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560 &&
+		 field->application != HID_GD_MOUSE)
+			return -1;
 
 	return 0;
 }
@@ -998,6 +1173,8 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
 
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		return wtp_raw_event(hidpp->hid_dev, data, size);
+	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
+		return m560_raw_event(hidpp->hid_dev, data, size);
 
 	return 0;
 }
@@ -1026,7 +1203,8 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
 
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		return wtp_raw_event(hdev, data, size);
-
+	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
+		return m560_raw_event(hidpp->hid_dev, data, size);
 	return 0;
 }
 
@@ -1100,6 +1278,8 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		wtp_connect(hdev, connected);
+	if ((hidpp->quirks & HIDPP_QUIRK_CLASS_M560) && connected)
+		m560_send_config_command(hdev);
 
 	if (!connected || hidpp->delayed_input)
 		return;
@@ -1162,7 +1342,12 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
 		ret = wtp_allocate(hdev, id);
 		if (ret)
-			goto wtp_allocate_fail;
+			goto allocate_fail;
+	}
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) {
+		ret = m560_allocate(hdev);
+		if (ret)
+			goto allocate_fail;
 	}
 
 	INIT_WORK(&hidpp->work, delayed_work_cb);
@@ -1228,7 +1413,7 @@ hid_hw_start_fail:
 hid_parse_fail:
 	cancel_work_sync(&hidpp->work);
 	mutex_destroy(&hidpp->send_mutex);
-wtp_allocate_fail:
+allocate_fail:
 	hid_set_drvdata(hdev, NULL);
 	return ret;
 }
@@ -1261,6 +1446,12 @@ static const struct hid_device_id hidpp_devices[] = {
 		USB_VENDOR_ID_LOGITECH, 0x4102),
 	  .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_MULTI_INPUT |
 			 HIDPP_QUIRK_CLASS_WTP },
+	{ /* Mouse logitech M560 */
+	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
+		USB_VENDOR_ID_LOGITECH, 0x402d),
+	  .driver_data = HIDPP_QUIRK_CLASS_M560 | HIDPP_QUIRK_DELAYED_INIT |
+			 HIDPP_QUIRK_MULTI_INPUT
+	},
 
 	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},

Re: [Patch V2] Driver for Logitech M560

[Antonio Ospite]

On Mon, 13 Apr 2015 20:24:05 +0200
Goffredo Baroncelli <kreijack@inwind.it> wrote:

> Hi Benjamin,
> 
> 
> I accepted near all your suggestions; the one which I leaved out is the
> one related to using input_event()/input_sync() instead of rewriting
> the hid report. I am not against to it, but... I don't know enough the hid
> stack, so I need some more suggestions from your side.
> 
> In the meantime, I updated the patch on the basis of your suggestions.
> 

I am adding just some syntactic remarks below, I haven't tried or
fully understood the code yet; just things to take in mind for when you
are going to submit the next iteration of the patch.

In the short commit message (Subject) use the subsystem/driver prefix
(use git log to figure it out), in this case it's something like:

	HID: logitech-hidpp: add support for Logitech M560
	
The introduction/annotation goes after the '---' marker; it is the
commit message which goes at the beginning of the email body so that
"git am" can pick it up and ignore the annotations.
Check out Documentations/SubmittingPatches.

If possible use "git format-patch" and "git send-email" to submit
kernel patches.

> 
> BR
> G.Baroncelli
> 
> -- 
> gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
> Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
> 
> ----
> 
> The Logitech M560 is is a mouse designed for windows 8. 
> Comparing to a standard one, some buttons (the middle one and the 
> two ones placed on the side) are bounded to a key combination 
> instead of a classic "mouse" button.
> 
> Think this mouse as a pair of mouse and keyboard. When the middle
> button is pressed the it sends a key (as keyboard) 
> combination, the same for the other two side button.
> Instead the left/right/wheel work correctly.
> To complicate further the things, the middle button send a
> key combination the odd press, and another one for the even press;
> in the latter case it sends also a left click. But the worst thing 
> is that no event is generated when the middle button is released.
> 
> Moreover this device is a wireless mouse which uses the unifying 
> receiver.
> 
> I discovered that it is possible to re-configure the mouse
> sending a command (see function m560_send_config_command()).
> After this command the mouse sends some sequence when the
> buttons are pressed and/or released (see comments for
> an explanation of the mouse protocol).
> 
> This patch update the file driver/hid/hid-logitech-hidpp.c (v3.19.3)

Comments like this are OK for the annotation but they are not that
useful in the commit message.

> 
> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
> 
> Changelog:
> - v1 first issue
> - v2 accepted Benjamin Tissoires suggestions
>

Patch history goes in the annotation section.

> diff --git a/hid-logitech-hidpp.c b/hid-logitech-hidpp.c

Submit patches against the full kernel tree.

> index a93cefe..805ecb6 100644
> --- a/hid-logitech-hidpp.c
> +++ b/hid-logitech-hidpp.c
> @@ -35,8 +35,9 @@ MODULE_AUTHOR("Nestor Lopez Casado <nlopezcasad@logitech.com>");
>  #define HIDPP_REPORT_LONG_LENGTH		20
>  
>  #define HIDPP_QUIRK_CLASS_WTP			BIT(0)
> +#define HIDPP_QUIRK_CLASS_M560			BIT(1)
>  
> -/* bits 1..20 are reserved for classes */
> +/* bits 2..20 are reserved for classes */
>  #define HIDPP_QUIRK_DELAYED_INIT		BIT(21)
>  #define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS	BIT(22)
>  #define HIDPP_QUIRK_MULTI_INPUT			BIT(23)
> @@ -925,6 +926,177 @@ static void wtp_connect(struct hid_device *hdev, bool connected)
>  }
>  
>  /* -------------------------------------------------------------------------- */
> +/* Logitech M560 devices                                                     */
> +/* -------------------------------------------------------------------------- */
> +
> +/*
> + * Logitech M560 protocol overview
> + *
> + * The Logitech M560 mouse, is designed for windows 8. When the middle and/or
> + * the sides buttons are pressed, it sends some keyboard keys events
> + * instead of buttons ones.
> + * To complicate further the things, the middle button keys sequence
> + * is different from the odd press and the even press.
> + *
> + * forward button -> Super_R
> + * backward button -> Super_L+'d' (press only)
> + * middle button -> 1st time: Alt_L+SuperL+XF86TouchpadOff (press only)
> + *                  2nd time: left-click (press only)
> + * NB: press-only means that when the button is pressed, the
> + * KeyPress/ButtonPress and KeyRelease/ButtonRelease events are generated
> + * together sequentially; instead when the button is released, no event is
> + * generated !
> + *
> + * With the command
> + *	10<xx>0a 3500af03 (where <xx> is the mouse id),
> + * the mouse reacts differently:
> + * - it never send a keyboard key event
> + * - for the three mouse button it sends:
> + *	middle button               press   11<xx>0a 3500af00...
> + *	side 1 button (forward)     press   11<xx>0a 3500b000...
> + *	side 2 button (backward)    press   11<xx>0a 3500ae00...
> + *	middle/side1/side2 button   release 11<xx>0a 35000000...
> + */
> +
> +static const u8 m560_config_parameter[] = {0x00, 0xaf, 0x03};
> +
> +struct m560_private_data {
> +	u8 prev_data[10];
> +	u8 button_pressed;
> +};
> +
> +/* how the button are mapped in the report */
> +#define M560_MOUSE_BTN_LEFT		0x01
> +#define M560_MOUSE_BTN_RIGHT		0x02
> +#define M560_MOUSE_BTN_MIDDLE		0x04
> +#define M560_MOUSE_BTN_WHEEL_LEFT	0x08
> +#define M560_MOUSE_BTN_WHEEL_RIGHT	0x10
> +#define M560_MOUSE_BTN_FORWARD		0x20
> +#define M560_MOUSE_BTN_BACKWARD		0x40
> +#define M560_SUB_ID			0x0a
> +#define M560_BUTTON_MODE_REGISTER	0x35
> +
> +/*
> + * m560_send_config_command - send the config_command to the mouse
> + *
> + * @dev: hid device where the mouse belongs
> + *
> + * @return: 0 OK
> + */
> +static int m560_send_config_command(struct hid_device *hdev)
> +{
> +	struct hidpp_report response;
> +	struct hidpp_device *hidpp_dev = hid_get_drvdata(hdev);
> +	int ret;
> +
> +	ret = hidpp_send_rap_command_sync(
> +		hidpp_dev,
> +		REPORT_ID_HIDPP_SHORT,
> +		M560_SUB_ID,
> +		M560_BUTTON_MODE_REGISTER,
> +		(u8 *)m560_config_parameter,
> +		sizeof(m560_config_parameter),
> +		&response
> +	);

kernel style is to wrap when needed, not earlier, but it's not a big
deal.

> +
> +	return ret;
> +}
> +
> +static int m560_allocate(struct hid_device *hdev)
> +{
> +	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> +	struct m560_private_data *d;
> +
> +	d = devm_kzalloc(&hdev->dev, sizeof(struct m560_private_data),
> +			GFP_KERNEL);
> +	if (!d)
> +		return -ENOMEM;
> +
> +	hidpp->private_data = d;
> +
> +	return 0;
> +};
> +
> +static int m560_raw_event(struct hid_device *hdev, u8 *data, int size)
> +{
> +	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> +	struct m560_private_data *mydata = hidpp->private_data;
> +	u8 *btn_ptr = data+1;
> +	u8 *wheel_ptr = data+1+6;

Put spaces around operators.

> +
> +	/* check if the data is a mouse related report */
> +	if (data[0] != 0x02 && data[2] != M560_SUB_ID)
> +		return 1;
> +
> +	if (data[0] == REPORT_ID_HIDPP_LONG &&
> +	    data[2] == M560_SUB_ID && data[06] == 0x00) {
                                           ^^
Index in in octal (06) :)

> +		/*
> +		 * m560 mouse button report
> +		 *
> +		 * data[0] = 0x11
> +		 * data[1] = deviceid
> +		 * data[2] = 0x0a
> +		 * data[5] = button (0xaf->middle, 0xb0->forward,
> +		 *                   0xaf ->backward, 0x00->release all)
> +		 * data[6] = 0x00
> +		 */
> +
> +		int btn, i, maxsize;
> +
> +		/* check if the event is a button */
> +		btn = data[5];
> +		if (btn != 0x00 && btn != 0xb0 && btn != 0xae && btn != 0xaf)
> +			return 1;
> +
> +		if (btn == 0xaf)
> +			mydata->button_pressed |= M560_MOUSE_BTN_MIDDLE;
> +		else if (btn == 0xb0)
> +			mydata->button_pressed |= M560_MOUSE_BTN_FORWARD;
> +		else if (btn == 0xae)
> +			mydata->button_pressed |= M560_MOUSE_BTN_BACKWARD;
> +		else if (btn == 0x00)
> +			mydata->button_pressed &= ~(M560_MOUSE_BTN_BACKWARD|
> +				M560_MOUSE_BTN_MIDDLE|M560_MOUSE_BTN_FORWARD);
> +
> +		/* replace the report with the old one */
> +		if (size > sizeof(mydata->prev_data))
> +			maxsize = sizeof(mydata->prev_data);
> +		else
> +			maxsize = size;

a blank line before the 'for' makes it stand out.

> +		for (i = 0 ; i < maxsize ; i++)
> +			data[i] = mydata->prev_data[i];
> +
> +	} else if (data[0] == 0x02) {
> +		/*
> +		 * standard mouse report
> +		 *
> +		 * data[0] = type (0x02)
> +		 * data[1..2] = buttons
> +		 * data[3..5] = xy
> +		 * data[6] = wheel
> +		 * data[7] = horizontal wheel
> +		 */
> +
> +		/* horizontal wheel handling */
> +		if (*btn_ptr & M560_MOUSE_BTN_WHEEL_LEFT)
> +			*wheel_ptr = -1;
> +		if (*btn_ptr & M560_MOUSE_BTN_WHEEL_RIGHT)
> +			*wheel_ptr =  1;

"else if" here? I am not sure.

> +
> +		*btn_ptr &= ~(M560_MOUSE_BTN_WHEEL_LEFT|
> +				M560_MOUSE_BTN_WHEEL_RIGHT);
> +
> +		/* copy the type and buttons status */
> +		memcpy(mydata->prev_data, data, 3);
> +	}
> +
> +	/* add the extra buttons */
> +	*btn_ptr |= mydata->button_pressed;
> +
> +	return 1;
> +}
> +
> +/* -------------------------------------------------------------------------- */
>  /* Generic HID++ devices                                                      */
>  /* -------------------------------------------------------------------------- */
>  
> @@ -936,6 +1108,9 @@ static int hidpp_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  
>  	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>  		return wtp_input_mapping(hdev, hi, field, usage, bit, max);
> +	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560 &&
> +		 field->application != HID_GD_MOUSE)
> +			return -1;

you could add a m560_input_mapping() here and do the application check
there, for symmetry with the current code.

>  
>  	return 0;
>  }
> @@ -998,6 +1173,8 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
>  
>  	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>  		return wtp_raw_event(hidpp->hid_dev, data, size);
> +	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
> +		return m560_raw_event(hidpp->hid_dev, data, size);
>  
>  	return 0;
>  }
> @@ -1026,7 +1203,8 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
>  
>  	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>  		return wtp_raw_event(hdev, data, size);
> -
> +	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
> +		return m560_raw_event(hidpp->hid_dev, data, size);

A blank line before the return wouldn't hurt.

>  	return 0;
>  }
>  
> @@ -1100,6 +1278,8 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
>  
>  	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>  		wtp_connect(hdev, connected);
> +	if ((hidpp->quirks & HIDPP_QUIRK_CLASS_M560) && connected)
> +		m560_send_config_command(hdev);

Maybe use "else if" here to highlight that these quirks are mutually
exclusive.

>  	if (!connected || hidpp->delayed_input)
>  		return;
> @@ -1162,7 +1342,12 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
>  		ret = wtp_allocate(hdev, id);
>  		if (ret)
> -			goto wtp_allocate_fail;
> +			goto allocate_fail;
> +	}
> +	if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) {

"else if" here too? More as a semantic hint than anything else, this
also gives less ambiguity to the 'allocate_fail' label.

> +		ret = m560_allocate(hdev);
> +		if (ret)
> +			goto allocate_fail;
>  	}
>  
>  	INIT_WORK(&hidpp->work, delayed_work_cb);
> @@ -1228,7 +1413,7 @@ hid_hw_start_fail:
>  hid_parse_fail:
>  	cancel_work_sync(&hidpp->work);
>  	mutex_destroy(&hidpp->send_mutex);
> -wtp_allocate_fail:
> +allocate_fail:
>  	hid_set_drvdata(hdev, NULL);
>  	return ret;
>  }
> @@ -1261,6 +1446,12 @@ static const struct hid_device_id hidpp_devices[] = {
>  		USB_VENDOR_ID_LOGITECH, 0x4102),
>  	  .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_MULTI_INPUT |
>  			 HIDPP_QUIRK_CLASS_WTP },
> +	{ /* Mouse logitech M560 */
> +	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
> +		USB_VENDOR_ID_LOGITECH, 0x402d),
> +	  .driver_data = HIDPP_QUIRK_CLASS_M560 | HIDPP_QUIRK_DELAYED_INIT |
> +			 HIDPP_QUIRK_MULTI_INPUT

Order quirks in a way similar to the other devices, for consistency,
i.e. HIDPP_QUIRK_CLASS_M560 may go last.

> +	},
>  
>  	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
>  		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
> 
> --

I'll let you know when we (me or Dario, in CC) actually test the code.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: [Patch V2] Driver for Logitech M560

[Dario Righelli]

Hi Goffredo,

I've tested your patch for Logitech M560 mouse on kernel v3.19.3 with
xev and evtest commands, it works well.
In this mail I report the output of evtest command.

Bye,
Dario

>
> ----
>
> The Logitech M560 is is a mouse designed for windows 8.
> Comparing to a standard one, some buttons (the middle one and the
> two ones placed on the side) are bounded to a key combination
> instead of a classic "mouse" button.
>
> Think this mouse as a pair of mouse and keyboard. When the middle
> button is pressed the it sends a key (as keyboard)
> combination, the same for the other two side button.
> Instead the left/right/wheel work correctly.
> To complicate further the things, the middle button send a
> key combination the odd press, and another one for the even press;
> in the latter case it sends also a left click. But the worst thing
> is that no event is generated when the middle button is released.
>
> Moreover this device is a wireless mouse which uses the unifying
> receiver.
>
> I discovered that it is possible to re-configure the mouse
> sending a command (see function m560_send_config_command()).
> After this command the mouse sends some sequence when the
> buttons are pressed and/or released (see comments for
> an explanation of the mouse protocol).
>
> This patch update the file driver/hid/hid-logitech-hidpp.c (v3.19.3)
>
> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>

Tested-by: Dario Righelli <drighelli@gmail.com>

$ sudo evtest
No device specified, trying to scan all of /dev/input/event*
Available devices:
/dev/input/event0:    Power Button
/dev/input/event1:    Power Button
/dev/input/event2:    USB USB Keyboard
/dev/input/event3:    USB USB Keyboard
/dev/input/event4:    Logitech USB Receiver
/dev/input/event5:    Logitech USB Receiver
/dev/input/event6:    Logitech M560
/dev/input/event7:    Logitech Wireless Mouse M560
Select the device event number [0-7]: 6
Input driver version is 1.0.1
Input device ID: bus 0x3 vendor 0x46d product 0x402d version 0x111
Input device name: "Logitech M560"
Supported events:
  Event type 0 (EV_SYN)
  Event type 1 (EV_KEY)
    Event code 272 (BTN_LEFT)
    Event code 273 (BTN_RIGHT)
    Event code 274 (BTN_MIDDLE)
    Event code 275 (BTN_SIDE)
    Event code 276 (BTN_EXTRA)
    Event code 277 (BTN_FORWARD)
    Event code 278 (BTN_BACK)
    Event code 279 (BTN_TASK)
    Event code 280 (?)
    Event code 281 (?)
    Event code 282 (?)
    Event code 283 (?)
    Event code 284 (?)
    Event code 285 (?)
    Event code 286 (?)
    Event code 287 (?)
  Event type 2 (EV_REL)
    Event code 0 (REL_X)
    Event code 1 (REL_Y)
    Event code 6 (REL_HWHEEL)
    Event code 8 (REL_WHEEL)
  Event type 4 (EV_MSC)
    Event code 4 (MSC_SCAN)

>
> Changelog:
> - v1 first issue
> - v2 accepted Benjamin Tissoires suggestions
>
> diff --git a/hid-logitech-hidpp.c b/hid-logitech-hidpp.c
> index a93cefe..805ecb6 100644
> --- a/hid-logitech-hidpp.c
> +++ b/hid-logitech-hidpp.c
> @@ -35,8 +35,9 @@ MODULE_AUTHOR("Nestor Lopez Casado <nlopezcasad@logitech.com>");
>  #define HIDPP_REPORT_LONG_LENGTH               20
>
>  #define HIDPP_QUIRK_CLASS_WTP                  BIT(0)
> +#define HIDPP_QUIRK_CLASS_M560                 BIT(1)
>
> -/* bits 1..20 are reserved for classes */
> +/* bits 2..20 are reserved for classes */
>  #define HIDPP_QUIRK_DELAYED_INIT               BIT(21)
>  #define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS       BIT(22)
>  #define HIDPP_QUIRK_MULTI_INPUT                        BIT(23)
> @@ -925,6 +926,177 @@ static void wtp_connect(struct hid_device *hdev, bool connected)
>  }
>
>  /* -------------------------------------------------------------------------- */
> +/* Logitech M560 devices                                                     */
> +/* -------------------------------------------------------------------------- */
> +
> +/*
> + * Logitech M560 protocol overview
> + *
> + * The Logitech M560 mouse, is designed for windows 8. When the middle and/or
> + * the sides buttons are pressed, it sends some keyboard keys events
> + * instead of buttons ones.
> + * To complicate further the things, the middle button keys sequence
> + * is different from the odd press and the even press.
> + *
> + * forward button -> Super_R
> + * backward button -> Super_L+'d' (press only)
> + * middle button -> 1st time: Alt_L+SuperL+XF86TouchpadOff (press only)
> + *                  2nd time: left-click (press only)
> + * NB: press-only means that when the button is pressed, the
> + * KeyPress/ButtonPress and KeyRelease/ButtonRelease events are generated
> + * together sequentially; instead when the button is released, no event is
> + * generated !
> + *
> + * With the command
> + *     10<xx>0a 3500af03 (where <xx> is the mouse id),
> + * the mouse reacts differently:
> + * - it never send a keyboard key event
> + * - for the three mouse button it sends:
> + *     middle button               press   11<xx>0a 3500af00...
> + *     side 1 button (forward)     press   11<xx>0a 3500b000...
> + *     side 2 button (backward)    press   11<xx>0a 3500ae00...
> + *     middle/side1/side2 button   release 11<xx>0a 35000000...
> + */
> +
> +static const u8 m560_config_parameter[] = {0x00, 0xaf, 0x03};
> +
> +struct m560_private_data {
> +       u8 prev_data[10];
> +       u8 button_pressed;
> +};
> +
> +/* how the button are mapped in the report */
> +#define M560_MOUSE_BTN_LEFT            0x01
> +#define M560_MOUSE_BTN_RIGHT           0x02
> +#define M560_MOUSE_BTN_MIDDLE          0x04
> +#define M560_MOUSE_BTN_WHEEL_LEFT      0x08
> +#define M560_MOUSE_BTN_WHEEL_RIGHT     0x10
> +#define M560_MOUSE_BTN_FORWARD         0x20
> +#define M560_MOUSE_BTN_BACKWARD                0x40
> +#define M560_SUB_ID                    0x0a
> +#define M560_BUTTON_MODE_REGISTER      0x35
> +
> +/*
> + * m560_send_config_command - send the config_command to the mouse
> + *
> + * @dev: hid device where the mouse belongs
> + *
> + * @return: 0 OK
> + */
> +static int m560_send_config_command(struct hid_device *hdev)
> +{
> +       struct hidpp_report response;
> +       struct hidpp_device *hidpp_dev = hid_get_drvdata(hdev);
> +       int ret;
> +
> +       ret = hidpp_send_rap_command_sync(
> +               hidpp_dev,
> +               REPORT_ID_HIDPP_SHORT,
> +               M560_SUB_ID,
> +               M560_BUTTON_MODE_REGISTER,
> +               (u8 *)m560_config_parameter,
> +               sizeof(m560_config_parameter),
> +               &response
> +       );
> +
> +       return ret;
> +}
> +
> +static int m560_allocate(struct hid_device *hdev)
> +{
> +       struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> +       struct m560_private_data *d;
> +
> +       d = devm_kzalloc(&hdev->dev, sizeof(struct m560_private_data),
> +                       GFP_KERNEL);
> +       if (!d)
> +               return -ENOMEM;
> +
> +       hidpp->private_data = d;
> +
> +       return 0;
> +};
> +
> +static int m560_raw_event(struct hid_device *hdev, u8 *data, int size)
> +{
> +       struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> +       struct m560_private_data *mydata = hidpp->private_data;
> +       u8 *btn_ptr = data+1;
> +       u8 *wheel_ptr = data+1+6;
> +
> +       /* check if the data is a mouse related report */
> +       if (data[0] != 0x02 && data[2] != M560_SUB_ID)
> +               return 1;
> +
> +       if (data[0] == REPORT_ID_HIDPP_LONG &&
> +           data[2] == M560_SUB_ID && data[06] == 0x00) {
> +               /*
> +                * m560 mouse button report
> +                *
> +                * data[0] = 0x11
> +                * data[1] = deviceid
> +                * data[2] = 0x0a
> +                * data[5] = button (0xaf->middle, 0xb0->forward,
> +                *                   0xaf ->backward, 0x00->release all)
> +                * data[6] = 0x00
> +                */
> +
> +               int btn, i, maxsize;
> +
> +               /* check if the event is a button */
> +               btn = data[5];
> +               if (btn != 0x00 && btn != 0xb0 && btn != 0xae && btn != 0xaf)
> +                       return 1;
> +
> +               if (btn == 0xaf)
> +                       mydata->button_pressed |= M560_MOUSE_BTN_MIDDLE;
> +               else if (btn == 0xb0)
> +                       mydata->button_pressed |= M560_MOUSE_BTN_FORWARD;
> +               else if (btn == 0xae)
> +                       mydata->button_pressed |= M560_MOUSE_BTN_BACKWARD;
> +               else if (btn == 0x00)
> +                       mydata->button_pressed &= ~(M560_MOUSE_BTN_BACKWARD|
> +                               M560_MOUSE_BTN_MIDDLE|M560_MOUSE_BTN_FORWARD);
> +
> +               /* replace the report with the old one */
> +               if (size > sizeof(mydata->prev_data))
> +                       maxsize = sizeof(mydata->prev_data);
> +               else
> +                       maxsize = size;
> +               for (i = 0 ; i < maxsize ; i++)
> +                       data[i] = mydata->prev_data[i];
> +
> +       } else if (data[0] == 0x02) {
> +               /*
> +                * standard mouse report
> +                *
> +                * data[0] = type (0x02)
> +                * data[1..2] = buttons
> +                * data[3..5] = xy
> +                * data[6] = wheel
> +                * data[7] = horizontal wheel
> +                */
> +
> +               /* horizontal wheel handling */
> +               if (*btn_ptr & M560_MOUSE_BTN_WHEEL_LEFT)
> +                       *wheel_ptr = -1;
> +               if (*btn_ptr & M560_MOUSE_BTN_WHEEL_RIGHT)
> +                       *wheel_ptr =  1;
> +
> +               *btn_ptr &= ~(M560_MOUSE_BTN_WHEEL_LEFT|
> +                               M560_MOUSE_BTN_WHEEL_RIGHT);
> +
> +               /* copy the type and buttons status */
> +               memcpy(mydata->prev_data, data, 3);
> +       }
> +
> +       /* add the extra buttons */
> +       *btn_ptr |= mydata->button_pressed;
> +
> +       return 1;
> +}
> +
> +/* -------------------------------------------------------------------------- */
>  /* Generic HID++ devices                                                      */
>  /* -------------------------------------------------------------------------- */
>
> @@ -936,6 +1108,9 @@ static int hidpp_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>
>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>                 return wtp_input_mapping(hdev, hi, field, usage, bit, max);
> +       else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560 &&
> +                field->application != HID_GD_MOUSE)
> +                       return -1;
>
>         return 0;
>  }
> @@ -998,6 +1173,8 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
>
>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>                 return wtp_raw_event(hidpp->hid_dev, data, size);
> +       else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
> +               return m560_raw_event(hidpp->hid_dev, data, size);
>
>         return 0;
>  }
> @@ -1026,7 +1203,8 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
>
>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>                 return wtp_raw_event(hdev, data, size);
> -
> +       else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
> +               return m560_raw_event(hidpp->hid_dev, data, size);
>         return 0;
>  }
>
> @@ -1100,6 +1278,8 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
>
>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>                 wtp_connect(hdev, connected);
> +       if ((hidpp->quirks & HIDPP_QUIRK_CLASS_M560) && connected)
> +               m560_send_config_command(hdev);
>
>         if (!connected || hidpp->delayed_input)
>                 return;
> @@ -1162,7 +1342,12 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
>                 ret = wtp_allocate(hdev, id);
>                 if (ret)
> -                       goto wtp_allocate_fail;
> +                       goto allocate_fail;
> +       }
> +       if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) {
> +               ret = m560_allocate(hdev);
> +               if (ret)
> +                       goto allocate_fail;
>         }
>
>         INIT_WORK(&hidpp->work, delayed_work_cb);
> @@ -1228,7 +1413,7 @@ hid_hw_start_fail:
>  hid_parse_fail:
>         cancel_work_sync(&hidpp->work);
>         mutex_destroy(&hidpp->send_mutex);
> -wtp_allocate_fail:
> +allocate_fail:
>         hid_set_drvdata(hdev, NULL);
>         return ret;
>  }
> @@ -1261,6 +1446,12 @@ static const struct hid_device_id hidpp_devices[] = {
>                 USB_VENDOR_ID_LOGITECH, 0x4102),
>           .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_MULTI_INPUT |
>                          HIDPP_QUIRK_CLASS_WTP },
> +       { /* Mouse logitech M560 */
> +         HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
> +               USB_VENDOR_ID_LOGITECH, 0x402d),
> +         .driver_data = HIDPP_QUIRK_CLASS_M560 | HIDPP_QUIRK_DELAYED_INIT |
> +                        HIDPP_QUIRK_MULTI_INPUT
> +       },
>
>         { HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
>                 USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
>

Re: Driver for Logitech M560

[Goffredo Baroncelli]

Hi Antonio,

I update my repository; I rebased my work on the current linux v3.19.3
kernel. The new repository is https://github.com/kreijack/logitech-m560.git 
(branch master).

If you want to make a dkms package for this driver I am interested to integrate


BR
G.Baroncelli

On 2015-04-09 14:41, Antonio Ospite wrote:
> On Fri, 05 Sep 2014 19:47:44 +0200
> Goffredo Baroncelli <kreijack@inwind.it> wrote:
> 
>> On 09/03/2014 11:36 PM, Benjamin Tissoires wrote:
>>> Hi Goffredo,
>>>
>>> On Mon, Sep 1, 2014 at 3:20 PM, Goffredo Baroncelli <kreijack@inwind.it> wrote:
>>>> Hi Benjamin,
>>>>
>>>> following the Nestor suggestion, I rewrote the driver for the
>>>> mouse Logitech M560 on the basis of your work (branch "for-whot").
>>>
>>> Just for the record. This branch is located here:
>>> https://github.com/bentiss/hid-logitech-dj/tree/for-whot and I
>>> *really* need to finish this work so that everything can go upstream
>>> :(
>>
> 
> Hi Benjamin, any news about upstreaming this work?
> 
>> :-) For me this is not a problem. I solved my issue (I made a dkms
>> package for this mouse), but I want to share my effort..
>>
> 
> Goffredo, is this the latest version of your dkms-enabled external
> module?
> https://github.com/kreijack/hid-logitech-dj/tree/m560-dkms
> 
> Thanks,
>    Antonio
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: Driver for Logitech M560

[Antonio Ospite]

On Sun, 12 Apr 2015 20:47:39 +0200
Goffredo Baroncelli <kreijack@inwind.it> wrote:

> Hi Antonio,
> 
> I update my repository; I rebased my work on the current linux v3.19.3
> kernel. The new repository is https://github.com/kreijack/logitech-m560.git 
> (branch master).
> 
> If you want to make a dkms package for this driver I am interested to integrate
> 

Hi Goffredo, I should be able to do some tests later this week.
I'll let you know.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?