[PATCH] Add driver for mouse logitech M560

[PATCH] Add driver for mouse logitech M560

[Goffredo Baroncelli]

From: Goffredo Baroncelli <kreijack@inwind.it>

The Logitech M560 is a wireless mouse designed for windows 8 which uses
the unifying receiver.
Compared to a standard one, some buttons (the middle one and the
two ones placed on the side) are bound to a key combination
instead of a generating classic "mouse" button events.

The device shows up as a mouse and keyboard combination: when the middle
button is pressed it sends a key (as keyboard) combination, the same
happens for the two side button. The left/right/wheel work as expected
from a mouse. To complicate things further, the middle button sends
different keys combinations between odd and even presses.
In the "even" press it also sends a left click. But the worst thing
is that no event is generated when the middle button is released.

It is possible to re-configure the mouse sending a command (see function
m560_send_config_command()). After this command the mouse sends some
useful data when the buttons are pressed and/or released.

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 drivers/hid/hid-logitech-hidpp.c | 242 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 239 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index b3cf6fd..e7dc23e 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -40,8 +40,9 @@ MODULE_PARM_DESC(disable_raw_mode,
 #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)
@@ -941,6 +942,220 @@ static int wtp_connect(struct hid_device *hdev, bool connected)
 			true, true);
 }
 
+static u32 hidpp_extract(u8 *report, unsigned offset, unsigned n)
+{
+	u64 x;
+
+	report += offset >> 3;  /* adjust byte index */
+	offset &= 7;            /* now only need bit offset into one byte */
+	x = get_unaligned_le64(report);
+	x = (x >> offset) & ((1ULL << n) - 1);  /* extract bit field */
+	return (u32)x;
+}
+
+/* ------------------------------------------------------------------------- */
+/* 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 things further, 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 {
+	struct input_dev *input;
+};
+
+/* 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
+
+static int m560_send_config_command(struct hid_device *hdev, bool connected)
+{
+	struct hidpp_report response;
+	struct hidpp_device *hidpp_dev = hid_get_drvdata(hdev);
+
+	if (!connected)
+		return -ENODEV;
+
+	return 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
+	);
+
+}
+
+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;
+
+
+	/* sanity check */
+	if (!mydata || !mydata->input)
+		return 1;
+	if (size < 7)
+		return 1;
+
+	/* exit if the data is not a mouse related report */
+	if (data[0] != 0x02 && data[2] != M560_SUB_ID)
+		return 0;
+
+	if (data[0] == REPORT_ID_HIDPP_LONG &&
+	    data[2] == M560_SUB_ID && data[6] == 0x00) {
+		/*
+		 * m560 mouse report for middle, forward and backward button
+		 *
+		 * data[0] = 0x11
+		 * data[1] = device-id
+		 * data[2] = 0x0a
+		 * data[5] = 0xaf -> middle
+		 *	     0xb0 -> forward
+		 *	     0xae -> backward
+		 *	     0x00 -> release all
+		 * data[6] = 0x00
+		 */
+
+		switch (data[5]) {
+		case 0xaf:
+			input_report_key(mydata->input, BTN_MIDDLE, 1);
+			break;
+		case 0xb0:
+			input_report_key(mydata->input, BTN_FORWARD, 1);
+			break;
+		case 0xae:
+			input_report_key(mydata->input, BTN_BACK, 1);
+			break;
+		case 0x00:
+			input_report_key(mydata->input, BTN_BACK, 0);
+			input_report_key(mydata->input, BTN_FORWARD, 0);
+			input_report_key(mydata->input, BTN_MIDDLE, 0);
+			break;
+		default:
+			return 1;
+		}
+		input_sync(mydata->input);
+
+	} else if (data[0] == 0x02) {
+		/*
+		 * Logitech M560 mouse report
+		 *
+		 * data[0] = type (0x02)
+		 * data[1..2] = buttons
+		 * data[3..5] = xy
+		 * data[6] = wheel
+		 */
+
+		int v;
+
+		input_report_key(mydata->input, BTN_LEFT,
+			!!(data[1] & M560_MOUSE_BTN_LEFT));
+		input_report_key(mydata->input, BTN_RIGHT,
+			!!(data[1] & M560_MOUSE_BTN_RIGHT));
+
+		if (data[1] & M560_MOUSE_BTN_WHEEL_LEFT)
+			input_report_rel(mydata->input, REL_HWHEEL, -1);
+		else if (data[1] & M560_MOUSE_BTN_WHEEL_RIGHT)
+			input_report_rel(mydata->input, REL_HWHEEL, 1);
+
+		v = hid_snto32(hidpp_extract(data+3, 0, 12), 12);
+		input_report_rel(mydata->input, REL_X, v);
+
+		v = hid_snto32(hidpp_extract(data+3, 12, 12), 12);
+		input_report_rel(mydata->input, REL_Y, v);
+
+		v = hid_snto32(data[6], 8);
+		input_report_rel(mydata->input, REL_WHEEL, v);
+
+		input_sync(mydata->input);
+	}
+
+	return 0;
+}
+
+static void m560_populate_input(struct hidpp_device *hidpp,
+		struct input_dev *input_dev, bool origin_is_hid_core)
+{
+	struct m560_private_data *mydata = hidpp->private_data;
+
+	mydata->input = input_dev;
+
+	__set_bit(EV_KEY, mydata->input->evbit);
+	__set_bit(BTN_MIDDLE, mydata->input->keybit);
+	__set_bit(BTN_RIGHT, mydata->input->keybit);
+	__set_bit(BTN_LEFT, mydata->input->keybit);
+	__set_bit(BTN_BACK, mydata->input->keybit);
+	__set_bit(BTN_FORWARD, mydata->input->keybit);
+
+	__set_bit(EV_REL, mydata->input->evbit);
+	__set_bit(REL_X, mydata->input->relbit);
+	__set_bit(REL_Y, mydata->input->relbit);
+	__set_bit(REL_WHEEL, mydata->input->relbit);
+	__set_bit(REL_HWHEEL, mydata->input->relbit);
+
+}
+
+static int m560_input_mapping(struct hid_device *hdev, struct hid_input *hi,
+		struct hid_field *field, struct hid_usage *usage,
+		unsigned long **bit, int *max)
+{
+	return -1;
+}
+
 /* -------------------------------------------------------------------------- */
 /* Generic HID++ devices                                                      */
 /* -------------------------------------------------------------------------- */
@@ -953,6 +1168,10 @@ 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 m560_input_mapping(hdev, hi, field, usage,
+						  bit, max);
 
 	return 0;
 }
@@ -962,6 +1181,8 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
 {
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		wtp_populate_input(hidpp, input, origin_is_hid_core);
+	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
+		m560_populate_input(hidpp, input, origin_is_hid_core);
 }
 
 static void hidpp_input_configured(struct hid_device *hdev,
@@ -1049,6 +1270,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(hdev, data, size);
 
 	return 0;
 }
@@ -1126,6 +1349,10 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 		ret = wtp_connect(hdev, connected);
 		if (ret)
 			return;
+	} else if ((hidpp->quirks & HIDPP_QUIRK_CLASS_M560) && connected) {
+		ret = m560_send_config_command(hdev, connected);
+		if (ret)
+			return;
 	}
 
 	if (!connected || hidpp->delayed_input)
@@ -1201,7 +1428,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);
@@ -1268,7 +1500,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;
 }
@@ -1301,6 +1533,10 @@ 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_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
 
 	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
-- 
2.1.4

[PATCH V4] Add support for mouse logitech m560

[Goffredo Baroncelli]

Hi All,

This is the 4th attempt for a Logitech mouse M560 hid driver.
In the previous attempt I didn't check carefully the arguments of the function
m560_raw_event() this leaded sometime to a system crash.
On the basis of the feedback received, I made some cleanup to the code.

The patch is against v4.1-rc1.

Special thanks to Antonio and Benjamin for their support and testing.

BR
G.Baroncelli


Changelog:
- v1 first issue
- v2 accepted Benjamin Tissoires suggestions
- v3 use input_report*() function instead of rewriting the hid-report
- v4 add check of the arguments in m560_raw_event()


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

Re: [PATCH] Add driver for mouse logitech M560

[Antonio Ospite]

On Fri,  1 May 2015 10:43:33 +0200
Goffredo Baroncelli <kreijack@libero.it> wrote:

> From: Goffredo Baroncelli <kreijack@inwind.it>
> 
> The Logitech M560 is a wireless mouse designed for windows 8 which uses
> the unifying receiver.
> Compared to a standard one, some buttons (the middle one and the
> two ones placed on the side) are bound to a key combination
> instead of a generating classic "mouse" button events.
> 
> The device shows up as a mouse and keyboard combination: when the middle
> button is pressed it sends a key (as keyboard) combination, the same
> happens for the two side button. The left/right/wheel work as expected
> from a mouse. To complicate things further, the middle button sends
> different keys combinations between odd and even presses.
> In the "even" press it also sends a left click. But the worst thing
> is that no event is generated when the middle button is released.
> 
> It is possible to re-configure the mouse sending a command (see function
> m560_send_config_command()). After this command the mouse sends some
> useful data when the buttons are pressed and/or released.
> 
> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>

Reviewed-by: Antonio Ospite <ao2@ao2.it>

Thanks a lot Goffredo, there are some final comments inlined below, they
are not blockers tho, and IMHO they could be addressed in future patches
if you didn't feel like re-submitting.

BTW in your future patches add the version suffix to the patch itself,
you can use the "--subject-prefix" option of git format-patch.

Annotations and patch history can go after the '---' marker as usual.

Sorry for not catching everything in one pass.

> ---
>  drivers/hid/hid-logitech-hidpp.c | 242 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 239 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index b3cf6fd..e7dc23e 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -40,8 +40,9 @@ MODULE_PARM_DESC(disable_raw_mode,
>  #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)
> @@ -941,6 +942,220 @@ static int wtp_connect(struct hid_device *hdev, bool connected)
>  			true, true);
>  }
>  

As Benjamin suggested, a comment saying that this is a duplicate of
drivers/hid/hid-core.c::extract() could be useful here.

> +static u32 hidpp_extract(u8 *report, unsigned offset, unsigned n)
> +{
> +	u64 x;
> +
> +	report += offset >> 3;  /* adjust byte index */
> +	offset &= 7;            /* now only need bit offset into one byte */
> +	x = get_unaligned_le64(report);
> +	x = (x >> offset) & ((1ULL << n) - 1);  /* extract bit field */
> +	return (u32)x;
> +}
> +
> +/* ------------------------------------------------------------------------- */
> +/* 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 things further, 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

it never sends ...
             ^
> + * - 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 {
> +	struct input_dev *input;
> +};
> +
> +/* how the button are mapped in the report */

how buttons are mapped ...
          ^
> +#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

These definitions for _MIDDLE, _FORWARD and _BACKWARD are not used
anymore in the code below. 

> +#define M560_SUB_ID			0x0a
> +#define M560_BUTTON_MODE_REGISTER	0x35
> +
> +static int m560_send_config_command(struct hid_device *hdev, bool connected)
> +{
> +	struct hidpp_report response;
> +	struct hidpp_device *hidpp_dev = hid_get_drvdata(hdev);
> +

hidpp_dev could be declared first, just to follow the style of the other
functions.

> +	if (!connected)
> +		return -ENODEV;
> +
> +	return 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
> +	);
> +

The empty line above can be dropped.

> +}
> +
> +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;
> +
> +

The extra empty line above can be dropped.

> +	/* sanity check */
> +	if (!mydata || !mydata->input)
> +		return 1;

And one empty line might be added here.

> +	if (size < 7)
> +		return 1;
> +
> +	/* exit if the data is not a mouse related report */
> +	if (data[0] != 0x02 && data[2] != M560_SUB_ID)

What are the possible values of data[0] that you observed?
Can it only be 0x02 and 0x11?

If we wanted to be stricter we could anticipate the full validation and
then simplify the ifs below to just check the report type, something
like:

	bool valid_mouse_report;
	...

	valid_mouse_report = (data[0] == 0x02 ||
	                      (data[0] == REPORT_ID_HIDPP_LONG &&
	                       data[2] == M560_SUB_ID &&
	                       data[6] == 0x00))

	if (!valid_mouse_report)
		return 0;

	if (data[0] == REPORT_ID_HIDPP_LONG) {
		...
	} else /* if (data[0] == 0x02) */ {
		...
	}

but maybe this is overkill.

> +		return 0;
> +
> +	if (data[0] == REPORT_ID_HIDPP_LONG &&
> +	    data[2] == M560_SUB_ID && data[6] == 0x00) {
> +		/*
> +		 * m560 mouse report for middle, forward and backward button
> +		 *
> +		 * data[0] = 0x11
> +		 * data[1] = device-id
> +		 * data[2] = 0x0a
> +		 * data[5] = 0xaf -> middle
> +		 *	     0xb0 -> forward
> +		 *	     0xae -> backward
> +		 *	     0x00 -> release all
> +		 * data[6] = 0x00
> +		 */
> +
> +		switch (data[5]) {
> +		case 0xaf:
> +			input_report_key(mydata->input, BTN_MIDDLE, 1);
> +			break;
> +		case 0xb0:
> +			input_report_key(mydata->input, BTN_FORWARD, 1);
> +			break;
> +		case 0xae:
> +			input_report_key(mydata->input, BTN_BACK, 1);
> +			break;
> +		case 0x00:
> +			input_report_key(mydata->input, BTN_BACK, 0);
> +			input_report_key(mydata->input, BTN_FORWARD, 0);
> +			input_report_key(mydata->input, BTN_MIDDLE, 0);
> +			break;

I am curios about the values of data[5] (and data[6]?) when multiple
buttons are pressed, it's a pity the hardware does not allow to combine
them consistently.

> +		default:
> +			return 1;
> +		}
> +		input_sync(mydata->input);
> +
> +	} else if (data[0] == 0x02) {
> +		/*
> +		 * Logitech M560 mouse report
> +		 *
> +		 * data[0] = type (0x02)
> +		 * data[1..2] = buttons
> +		 * data[3..5] = xy
> +		 * data[6] = wheel
> +		 */
> +
> +		int v;
> +
> +		input_report_key(mydata->input, BTN_LEFT,
> +			!!(data[1] & M560_MOUSE_BTN_LEFT));
> +		input_report_key(mydata->input, BTN_RIGHT,
> +			!!(data[1] & M560_MOUSE_BTN_RIGHT));
> +
> +		if (data[1] & M560_MOUSE_BTN_WHEEL_LEFT)
> +			input_report_rel(mydata->input, REL_HWHEEL, -1);
> +		else if (data[1] & M560_MOUSE_BTN_WHEEL_RIGHT)
> +			input_report_rel(mydata->input, REL_HWHEEL, 1);
> +
> +		v = hid_snto32(hidpp_extract(data+3, 0, 12), 12);
> +		input_report_rel(mydata->input, REL_X, v);
> +
> +		v = hid_snto32(hidpp_extract(data+3, 12, 12), 12);
> +		input_report_rel(mydata->input, REL_Y, v);
> +
> +		v = hid_snto32(data[6], 8);
> +		input_report_rel(mydata->input, REL_WHEEL, v);
> +
> +		input_sync(mydata->input);
> +	}
> +
> +	return 0;
> +}
> +
> +static void m560_populate_input(struct hidpp_device *hidpp,
> +		struct input_dev *input_dev, bool origin_is_hid_core)
> +{
> +	struct m560_private_data *mydata = hidpp->private_data;
> +
> +	mydata->input = input_dev;
> +
> +	__set_bit(EV_KEY, mydata->input->evbit);
> +	__set_bit(BTN_MIDDLE, mydata->input->keybit);
> +	__set_bit(BTN_RIGHT, mydata->input->keybit);
> +	__set_bit(BTN_LEFT, mydata->input->keybit);
> +	__set_bit(BTN_BACK, mydata->input->keybit);
> +	__set_bit(BTN_FORWARD, mydata->input->keybit);
> +
> +	__set_bit(EV_REL, mydata->input->evbit);
> +	__set_bit(REL_X, mydata->input->relbit);
> +	__set_bit(REL_Y, mydata->input->relbit);
> +	__set_bit(REL_WHEEL, mydata->input->relbit);
> +	__set_bit(REL_HWHEEL, mydata->input->relbit);
> +

The empty line above can be dropped.

> +}
> +
> +static int m560_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> +		struct hid_field *field, struct hid_usage *usage,
> +		unsigned long **bit, int *max)
> +{
> +	return -1;
> +}
> +
>  /* -------------------------------------------------------------------------- */
>  /* Generic HID++ devices                                                      */
>  /* -------------------------------------------------------------------------- */
> @@ -953,6 +1168,10 @@ 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 m560_input_mapping(hdev, hi, field, usage,
> +						  bit, max);

To be pedantic, the return above has one extra indentation level, it
should be at the same level as the one in the other branch.

>  
>  	return 0;
>  }
> @@ -962,6 +1181,8 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
>  {
>  	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>  		wtp_populate_input(hidpp, input, origin_is_hid_core);
> +	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
> +		m560_populate_input(hidpp, input, origin_is_hid_core);
>  }
>  
>  static void hidpp_input_configured(struct hid_device *hdev,
> @@ -1049,6 +1270,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(hdev, data, size);
>  
>  	return 0;
>  }
> @@ -1126,6 +1349,10 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
>  		ret = wtp_connect(hdev, connected);
>  		if (ret)
>  			return;
> +	} else if ((hidpp->quirks & HIDPP_QUIRK_CLASS_M560) && connected) {

No need to check for connected here anymore, the check is now in
m560_send_config_command()

> +		ret = m560_send_config_command(hdev, connected);
> +		if (ret)
> +			return;
>  	}
>  
>  	if (!connected || hidpp->delayed_input)
> @@ -1201,7 +1428,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?

> +		ret = m560_allocate(hdev);
> +		if (ret)
> +			goto allocate_fail;
>  	}
>  
>  	INIT_WORK(&hidpp->work, delayed_work_cb);
> @@ -1268,7 +1500,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;
>  }
> @@ -1301,6 +1533,10 @@ 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_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
>  
>  	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
>  		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
> -- 
> 2.1.4
> 


-- 
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] Add driver for mouse logitech M560

[Antonio Ospite]

On Tue, 5 May 2015 17:36:59 +0200
Antonio Ospite <ao2@ao2.it> wrote:

> On Fri,  1 May 2015 10:43:33 +0200
> Goffredo Baroncelli <kreijack@libero.it> wrote:
> 
[...]
> > +	/* exit if the data is not a mouse related report */
> > +	if (data[0] != 0x02 && data[2] != M560_SUB_ID)
> 
> What are the possible values of data[0] that you observed?
> Can it only be 0x02 and 0x11?
> 
> If we wanted to be stricter we could anticipate the full validation and
> then simplify the ifs below to just check the report type, something
> like:
> 
> 	bool valid_mouse_report;
> 	...
> 
> 	valid_mouse_report = (data[0] == 0x02 ||
> 	                      (data[0] == REPORT_ID_HIDPP_LONG &&
> 	                       data[2] == M560_SUB_ID &&
> 	                       data[6] == 0x00))
> 
> 	if (!valid_mouse_report)
> 		return 0;
> 
> 	if (data[0] == REPORT_ID_HIDPP_LONG) {
> 		...
> 	} else /* if (data[0] == 0x02) */ {
> 		...
> 	}
> 
> but maybe this is overkill.
> 

Or maybe just using an else, without the preventive validation, would
already improve readability, checking things only once:

	if (data[0] == REPORT_ID_HIDPP_LONG &&
	    data[2] == M560_SUB_ID && data[6] == 0x00) {
		...
	} else if (data[0] == 0x02) {
		...
	} else {
		/* unsupported report */
		return 0;
	}

But again, this is just a suggestion.

Ciao,
   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] Add driver for mouse logitech M560

[Antonio Ospite]

On Wed, 6 May 2015 08:48:14 +0200
Antonio Ospite <ao2@ao2.it> wrote:

> On Tue, 5 May 2015 17:36:59 +0200
> Antonio Ospite <ao2@ao2.it> wrote:
> 
> > On Fri,  1 May 2015 10:43:33 +0200
> > Goffredo Baroncelli <kreijack@libero.it> wrote:
> > 
> [...]
> > > +	/* exit if the data is not a mouse related report */
> > > +	if (data[0] != 0x02 && data[2] != M560_SUB_ID)
> > 
> > What are the possible values of data[0] that you observed?
> > Can it only be 0x02 and 0x11?
> > 
> > If we wanted to be stricter we could anticipate the full validation and
> > then simplify the ifs below to just check the report type, something
> > like:
> > 
> > 	bool valid_mouse_report;
> > 	...
> > 
> > 	valid_mouse_report = (data[0] == 0x02 ||
> > 	                      (data[0] == REPORT_ID_HIDPP_LONG &&
> > 	                       data[2] == M560_SUB_ID &&
> > 	                       data[6] == 0x00))
> > 
> > 	if (!valid_mouse_report)
> > 		return 0;
> > 
> > 	if (data[0] == REPORT_ID_HIDPP_LONG) {
> > 		...
> > 	} else /* if (data[0] == 0x02) */ {
> > 		...
> > 	}
> > 
> > but maybe this is overkill.
> > 
> 
> Or maybe just using an else, without the preventive validation, would
> already improve readability, checking things only once:
> 
> 	if (data[0] == REPORT_ID_HIDPP_LONG &&
> 	    data[2] == M560_SUB_ID && data[6] == 0x00) {
> 		...
> 	} else if (data[0] == 0x02) {
> 		...
> 	} else {
> 		/* unsupported report */
> 		return 0;
> 	}
> 

If the function returns 0 at the end, then the else here is even
redundant.

We may return 1 at the end of the function to differentiate between "no
action performed (0)" and "no further processing (1)" like stated in
include/linux/hid.h line 661, even if I don't see the core handling the
difference between the two return values when calling the raw_event
callback.

Benjamin, can you comment about what the convention is here? I am
referring also to the checks at the begin of m560_raw_event().

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] Add driver for mouse logitech M560

[Benjamin Tissoires]

On Wed, May 6, 2015 at 3:35 AM, Antonio Ospite <ao2@ao2.it> wrote:
> On Wed, 6 May 2015 08:48:14 +0200
> Antonio Ospite <ao2@ao2.it> wrote:
>
>> On Tue, 5 May 2015 17:36:59 +0200
>> Antonio Ospite <ao2@ao2.it> wrote:
>>
>> > On Fri,  1 May 2015 10:43:33 +0200
>> > Goffredo Baroncelli <kreijack@libero.it> wrote:
>> >
>> [...]
>> > > + /* exit if the data is not a mouse related report */
>> > > + if (data[0] != 0x02 && data[2] != M560_SUB_ID)
>> >
>> > What are the possible values of data[0] that you observed?
>> > Can it only be 0x02 and 0x11?
>> >
>> > If we wanted to be stricter we could anticipate the full validation and
>> > then simplify the ifs below to just check the report type, something
>> > like:
>> >
>> >     bool valid_mouse_report;
>> >     ...
>> >
>> >     valid_mouse_report = (data[0] == 0x02 ||
>> >                           (data[0] == REPORT_ID_HIDPP_LONG &&
>> >                            data[2] == M560_SUB_ID &&
>> >                            data[6] == 0x00))
>> >
>> >     if (!valid_mouse_report)
>> >             return 0;
>> >
>> >     if (data[0] == REPORT_ID_HIDPP_LONG) {
>> >             ...
>> >     } else /* if (data[0] == 0x02) */ {
>> >             ...
>> >     }
>> >
>> > but maybe this is overkill.
>> >
>>
>> Or maybe just using an else, without the preventive validation, would
>> already improve readability, checking things only once:
>>
>>       if (data[0] == REPORT_ID_HIDPP_LONG &&
>>           data[2] == M560_SUB_ID && data[6] == 0x00) {
>>               ...
>>       } else if (data[0] == 0x02) {
>>               ...
>>       } else {
>>               /* unsupported report */
>>               return 0;
>>       }
>>
>
> If the function returns 0 at the end, then the else here is even
> redundant.
>
> We may return 1 at the end of the function to differentiate between "no
> action performed (0)" and "no further processing (1)" like stated in
> include/linux/hid.h line 661, even if I don't see the core handling the
> difference between the two return values when calling the raw_event
> callback.
>
> Benjamin, can you comment about what the convention is here? I am

As you saw, we used to care, but we don't anymore. "no further
processing (1)" is now simply ignored and nobody took the time to fix
it properly. The last change which introduced that was b1a1442 ("HID:
core: fix reporting of raw events"), and it fixed a real problem. But
that means that we simply ignore the positive return value.

There may be a chance that we fix that in the future (I have too much
on my plate right now), so I would advice to follow the documentation
even if it is not entirely true anymore.

> referring also to the checks at the begin of m560_raw_event().

Again, there is no strict rule. The less operations you do, the better
it is (it will be called at each report, so potentially can add a lot
of head over).  Also the clearer it is, the better. That's why I am
often tempted to add extra functions for each type of processing with
a clear "return m560_process_hidpp_report()". Something like that :)

>
> Thanks,
>    Antonio

Cheers,
Benjamin

Re: [PATCH] Add driver for mouse logitech M560

[Goffredo Baroncelli]

Hi Jiri,
On 2015-05-29 16:44, Jiri Kosina wrote:
> On Fri, 29 May 2015, Benjamin Tissoires wrote:
> 
>>>> +static u32 hidpp_extract(u8 *report, unsigned offset, unsigned n)
>>>> +{
>>>> +     u64 x;
>>>> +
>>>> +     report += offset >> 3;  /* adjust byte index */
>>>> +     offset &= 7;            /* now only need bit offset into one byte */
>>>> +     x = get_unaligned_le64(report);
>>>> +     x = (x >> offset) & ((1ULL << n) - 1);  /* extract bit field */
>>>> +     return (u32)x;
>>>> +}
>>>
>>> I hate such code duplication. How about we rename it to
>>> hid_field_extract() and make its linkage external?
>>
>> works for me
> 
> Thanks. So I'd prefer this patch to be resent as a two-patch series, first 
> one bringing hid_field_extract(), and the second being the driver making 
> use of it.

I will update the patch on the basis of your request



> 
>>> [ ... snip ... ]
>>>> @@ -1301,6 +1532,10 @@ 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_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
>>>
>>> Seems like you forgot to add the device id to hid_have_special_driver[]?
>>
>> nope, the device is tagged with HID_GROUP_LOGITECH_DJ_DEVICE, so
>> hid-generic ignores it by default.
> 
> Gah, I keep forgetting about HID_GROUP_LOGITECH_DJ_DEVICE again and again.
> 
> Thanks,
> 


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

Re: [PATCH] Add driver for mouse logitech M560

[Jiri Kosina]

On Fri, 29 May 2015, Benjamin Tissoires wrote:

> >> +static u32 hidpp_extract(u8 *report, unsigned offset, unsigned n)
> >> +{
> >> +     u64 x;
> >> +
> >> +     report += offset >> 3;  /* adjust byte index */
> >> +     offset &= 7;            /* now only need bit offset into one byte */
> >> +     x = get_unaligned_le64(report);
> >> +     x = (x >> offset) & ((1ULL << n) - 1);  /* extract bit field */
> >> +     return (u32)x;
> >> +}
> >
> > I hate such code duplication. How about we rename it to
> > hid_field_extract() and make its linkage external?
> 
> works for me

Thanks. So I'd prefer this patch to be resent as a two-patch series, first 
one bringing hid_field_extract(), and the second being the driver making 
use of it.

> > [ ... snip ... ]
> >> @@ -1301,6 +1532,10 @@ 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_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
> >
> > Seems like you forgot to add the device id to hid_have_special_driver[]?
> 
> nope, the device is tagged with HID_GROUP_LOGITECH_DJ_DEVICE, so
> hid-generic ignores it by default.

Gah, I keep forgetting about HID_GROUP_LOGITECH_DJ_DEVICE again and again.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] Add driver for mouse logitech M560

[Benjamin Tissoires]

On Fri, May 29, 2015 at 10:38 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Sun, 10 May 2015, Goffredo Baroncelli wrote:
>
>> From: Goffredo Baroncelli <kreijack@inwind.it>
>>
>> The Logitech M560 is a wireless mouse designed for windows 8 which uses
>> the unifying receiver.
>> Compared to a standard one, some buttons (the middle one and the
>> two ones placed on the side) are bound to a key combination
>> instead of a generating classic "mouse" button events.
>>
>> The device shows up as a mouse and keyboard combination: when the middle
>> button is pressed it sends a key (as keyboard) combination, the same
>> happens for the two side button. The left/right/wheel work as expected
>> from a mouse. To complicate things further, the middle button sends
>> different keys combinations between odd and even presses.
>> In the "even" press it also sends a left click. But the worst thing
>> is that no event is generated when the middle button is released.
>>
>> It is possible to re-configure the mouse sending a command (see function
>> m560_send_config_command()). After this command the mouse sends some
>> useful data when the buttons are pressed and/or released.
>>
>> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
>> ---
>>  drivers/hid/hid-logitech-hidpp.c | 241 ++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 238 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
>> index b3cf6fd..2275f2a 100644
>> --- a/drivers/hid/hid-logitech-hidpp.c
>> +++ b/drivers/hid/hid-logitech-hidpp.c
>> @@ -40,8 +40,9 @@ MODULE_PARM_DESC(disable_raw_mode,
>>  #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)
>> @@ -941,6 +942,221 @@ static int wtp_connect(struct hid_device *hdev, bool connected)
>>                       true, true);
>>  }
>>
>> +/*
>> + * copied from hid-core.c
>> + */
>> +static u32 hidpp_extract(u8 *report, unsigned offset, unsigned n)
>> +{
>> +     u64 x;
>> +
>> +     report += offset >> 3;  /* adjust byte index */
>> +     offset &= 7;            /* now only need bit offset into one byte */
>> +     x = get_unaligned_le64(report);
>> +     x = (x >> offset) & ((1ULL << n) - 1);  /* extract bit field */
>> +     return (u32)x;
>> +}
>
> I hate such code duplication. How about we rename it to
> hid_field_extract() and make its linkage external?

works for me

>
> [ ... snip ... ]
>> @@ -1301,6 +1532,10 @@ 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_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
>
> Seems like you forgot to add the device id to hid_have_special_driver[]?

nope, the device is tagged with HID_GROUP_LOGITECH_DJ_DEVICE, so
hid-generic ignores it by default.

Cheers,
Benjamin

Re: [PATCH] Add driver for mouse logitech M560

[Jiri Kosina]

On Sun, 10 May 2015, Goffredo Baroncelli wrote:

> From: Goffredo Baroncelli <kreijack@inwind.it>
> 
> The Logitech M560 is a wireless mouse designed for windows 8 which uses
> the unifying receiver.
> Compared to a standard one, some buttons (the middle one and the
> two ones placed on the side) are bound to a key combination
> instead of a generating classic "mouse" button events.
> 
> The device shows up as a mouse and keyboard combination: when the middle
> button is pressed it sends a key (as keyboard) combination, the same
> happens for the two side button. The left/right/wheel work as expected
> from a mouse. To complicate things further, the middle button sends
> different keys combinations between odd and even presses.
> In the "even" press it also sends a left click. But the worst thing
> is that no event is generated when the middle button is released.
> 
> It is possible to re-configure the mouse sending a command (see function
> m560_send_config_command()). After this command the mouse sends some
> useful data when the buttons are pressed and/or released.
> 
> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
> ---
>  drivers/hid/hid-logitech-hidpp.c | 241 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 238 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index b3cf6fd..2275f2a 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -40,8 +40,9 @@ MODULE_PARM_DESC(disable_raw_mode,
>  #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)
> @@ -941,6 +942,221 @@ static int wtp_connect(struct hid_device *hdev, bool connected)
>  			true, true);
>  }
>  
> +/*
> + * copied from hid-core.c
> + */
> +static u32 hidpp_extract(u8 *report, unsigned offset, unsigned n)
> +{
> +	u64 x;
> +
> +	report += offset >> 3;  /* adjust byte index */
> +	offset &= 7;            /* now only need bit offset into one byte */
> +	x = get_unaligned_le64(report);
> +	x = (x >> offset) & ((1ULL << n) - 1);  /* extract bit field */
> +	return (u32)x;
> +}

I hate such code duplication. How about we rename it to 
hid_field_extract() and make its linkage external?

[ ... snip ... ]
> @@ -1301,6 +1532,10 @@ 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_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },

Seems like you forgot to add the device id to hid_have_special_driver[]?

Otherwise it looks good.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] Add driver for mouse logitech M560

[Benjamin Tissoires]

On Thu, May 21, 2015 at 3:05 AM, Goffredo Baroncelli <kreijack@libero.it> wrote:
> On 2015-05-11 17:28, Benjamin Tissoires wrote:
>> On Sun, May 10, 2015 at 12:49 PM, Goffredo Baroncelli
>> <kreijack@libero.it> wrote:
>>> From: Goffredo Baroncelli <kreijack@inwind.it>
>>>
> [....]
>>>
>>> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
>>> ---
>>
>> Works for me:
>>
>> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
>
> Hi Benjamin,
> who will take care to including this patch ?  You or Jiri Kosima ?

It will be Jiri.

> I asked that because in the code there is your name, but Antonio Ospite told me
> that Jiri Kosima would push the patch, but I never put him in CC....

I think that's fine - though he might ask for an other version, that
would be your punishment! :)

Jiri, would you mind having a look at Goffredo submission?

Cheers,
Benjamin

>
> BR
> G.Baroncelli
>
>>
>> Thanks!
>> Benjamin
>>
>>>  drivers/hid/hid-logitech-hidpp.c | 241 ++++++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 238 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
>>> index b3cf6fd..2275f2a 100644
>>> --- a/drivers/hid/hid-logitech-hidpp.c
>>> +++ b/drivers/hid/hid-logitech-hidpp.c
>>> @@ -40,8 +40,9 @@ MODULE_PARM_DESC(disable_raw_mode,
>>>  #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)
>>> @@ -941,6 +942,221 @@ static int wtp_connect(struct hid_device *hdev, bool connected)
>>>                         true, true);
>>>  }
>>>
>>> +/*
>>> + * copied from hid-core.c
>>> + */
>>> +static u32 hidpp_extract(u8 *report, unsigned offset, unsigned n)
>>> +{
>>> +       u64 x;
>>> +
>>> +       report += offset >> 3;  /* adjust byte index */
>>> +       offset &= 7;            /* now only need bit offset into one byte */
>>> +       x = get_unaligned_le64(report);
>>> +       x = (x >> offset) & ((1ULL << n) - 1);  /* extract bit field */
>>> +       return (u32)x;
>>> +}
>>> +
>>> +/* ------------------------------------------------------------------------- */
>>> +/* 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 things further, 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 sends 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 {
>>> +       struct input_dev *input;
>>> +};
>>> +
>>> +/* how buttons are mapped in the report */
>>> +#define M560_MOUSE_BTN_LEFT            0x01
>>> +#define M560_MOUSE_BTN_RIGHT           0x02
>>> +#define M560_MOUSE_BTN_WHEEL_LEFT      0x08
>>> +#define M560_MOUSE_BTN_WHEEL_RIGHT     0x10
>>> +
>>> +#define M560_SUB_ID                    0x0a
>>> +#define M560_BUTTON_MODE_REGISTER      0x35
>>> +
>>> +static int m560_send_config_command(struct hid_device *hdev, bool connected)
>>> +{
>>> +       struct hidpp_report response;
>>> +       struct hidpp_device *hidpp_dev;
>>> +
>>> +       hidpp_dev = hid_get_drvdata(hdev);
>>> +
>>> +       if (!connected)
>>> +               return -ENODEV;
>>> +
>>> +       return 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
>>> +       );
>>> +}
>>> +
>>> +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;
>>> +
>>> +       /* sanity check */
>>> +       if (!mydata || !mydata->input) {
>>> +               hid_err(hdev, "error in parameter\n");
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       if (size < 7) {
>>> +               hid_err(hdev, "error in report\n");
>>> +               return 0;
>>> +       }
>>> +
>>> +       if (data[0] == REPORT_ID_HIDPP_LONG &&
>>> +           data[2] == M560_SUB_ID && data[6] == 0x00) {
>>> +               /*
>>> +                * m560 mouse report for middle, forward and backward button
>>> +                *
>>> +                * data[0] = 0x11
>>> +                * data[1] = device-id
>>> +                * data[2] = 0x0a
>>> +                * data[5] = 0xaf -> middle
>>> +                *           0xb0 -> forward
>>> +                *           0xae -> backward
>>> +                *           0x00 -> release all
>>> +                * data[6] = 0x00
>>> +                */
>>> +
>>> +               switch (data[5]) {
>>> +               case 0xaf:
>>> +                       input_report_key(mydata->input, BTN_MIDDLE, 1);
>>> +                       break;
>>> +               case 0xb0:
>>> +                       input_report_key(mydata->input, BTN_FORWARD, 1);
>>> +                       break;
>>> +               case 0xae:
>>> +                       input_report_key(mydata->input, BTN_BACK, 1);
>>> +                       break;
>>> +               case 0x00:
>>> +                       input_report_key(mydata->input, BTN_BACK, 0);
>>> +                       input_report_key(mydata->input, BTN_FORWARD, 0);
>>> +                       input_report_key(mydata->input, BTN_MIDDLE, 0);
>>> +                       break;
>>> +               default:
>>> +                       hid_err(hdev, "error in report\n");
>>> +                       return 0;
>>> +               }
>>> +               input_sync(mydata->input);
>>> +
>>> +       } else if (data[0] == 0x02) {
>>> +               /*
>>> +                * Logitech M560 mouse report
>>> +                *
>>> +                * data[0] = type (0x02)
>>> +                * data[1..2] = buttons
>>> +                * data[3..5] = xy
>>> +                * data[6] = wheel
>>> +                */
>>> +
>>> +               int v;
>>> +
>>> +               input_report_key(mydata->input, BTN_LEFT,
>>> +                       !!(data[1] & M560_MOUSE_BTN_LEFT));
>>> +               input_report_key(mydata->input, BTN_RIGHT,
>>> +                       !!(data[1] & M560_MOUSE_BTN_RIGHT));
>>> +
>>> +               if (data[1] & M560_MOUSE_BTN_WHEEL_LEFT)
>>> +                       input_report_rel(mydata->input, REL_HWHEEL, -1);
>>> +               else if (data[1] & M560_MOUSE_BTN_WHEEL_RIGHT)
>>> +                       input_report_rel(mydata->input, REL_HWHEEL, 1);
>>> +
>>> +               v = hid_snto32(hidpp_extract(data+3, 0, 12), 12);
>>> +               input_report_rel(mydata->input, REL_X, v);
>>> +
>>> +               v = hid_snto32(hidpp_extract(data+3, 12, 12), 12);
>>> +               input_report_rel(mydata->input, REL_Y, v);
>>> +
>>> +               v = hid_snto32(data[6], 8);
>>> +               input_report_rel(mydata->input, REL_WHEEL, v);
>>> +
>>> +               input_sync(mydata->input);
>>> +       }
>>> +
>>> +       return 1;
>>> +}
>>> +
>>> +static void m560_populate_input(struct hidpp_device *hidpp,
>>> +               struct input_dev *input_dev, bool origin_is_hid_core)
>>> +{
>>> +       struct m560_private_data *mydata = hidpp->private_data;
>>> +
>>> +       mydata->input = input_dev;
>>> +
>>> +       __set_bit(EV_KEY, mydata->input->evbit);
>>> +       __set_bit(BTN_MIDDLE, mydata->input->keybit);
>>> +       __set_bit(BTN_RIGHT, mydata->input->keybit);
>>> +       __set_bit(BTN_LEFT, mydata->input->keybit);
>>> +       __set_bit(BTN_BACK, mydata->input->keybit);
>>> +       __set_bit(BTN_FORWARD, mydata->input->keybit);
>>> +
>>> +       __set_bit(EV_REL, mydata->input->evbit);
>>> +       __set_bit(REL_X, mydata->input->relbit);
>>> +       __set_bit(REL_Y, mydata->input->relbit);
>>> +       __set_bit(REL_WHEEL, mydata->input->relbit);
>>> +       __set_bit(REL_HWHEEL, mydata->input->relbit);
>>> +}
>>> +
>>> +static int m560_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>>> +               struct hid_field *field, struct hid_usage *usage,
>>> +               unsigned long **bit, int *max)
>>> +{
>>> +       return -1;
>>> +}
>>> +
>>>  /* -------------------------------------------------------------------------- */
>>>  /* Generic HID++ devices                                                      */
>>>  /* -------------------------------------------------------------------------- */
>>> @@ -953,6 +1169,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 m560_input_mapping(hdev, hi, field, usage, bit, max);
>>>
>>>         return 0;
>>>  }
>>> @@ -962,6 +1181,8 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
>>>  {
>>>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>>>                 wtp_populate_input(hidpp, input, origin_is_hid_core);
>>> +       else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
>>> +               m560_populate_input(hidpp, input, origin_is_hid_core);
>>>  }
>>>
>>>  static void hidpp_input_configured(struct hid_device *hdev,
>>> @@ -1049,6 +1270,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(hdev, data, size);
>>>
>>>         return 0;
>>>  }
>>> @@ -1126,6 +1349,10 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
>>>                 ret = wtp_connect(hdev, connected);
>>>                 if (ret)
>>>                         return;
>>> +       } else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) {
>>> +               ret = m560_send_config_command(hdev, connected);
>>> +               if (ret)
>>> +                       return;
>>>         }
>>>
>>>         if (!connected || hidpp->delayed_input)
>>> @@ -1201,7 +1428,11 @@ 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;
>>> +       } else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) {
>>> +               ret = m560_allocate(hdev);
>>> +               if (ret)
>>> +                       goto allocate_fail;
>>>         }
>>>
>>>         INIT_WORK(&hidpp->work, delayed_work_cb);
>>> @@ -1268,7 +1499,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;
>>>  }
>>> @@ -1301,6 +1532,10 @@ 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_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
>>>
>>>         { HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
>>>                 USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
>>> --
>>> 2.1.4
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
>
> --
> gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
> Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH] Add driver for mouse logitech M560

[Benjamin Tissoires]

On Sun, May 10, 2015 at 12:49 PM, Goffredo Baroncelli
<kreijack@libero.it> wrote:
> From: Goffredo Baroncelli <kreijack@inwind.it>
>
> The Logitech M560 is a wireless mouse designed for windows 8 which uses
> the unifying receiver.
> Compared to a standard one, some buttons (the middle one and the
> two ones placed on the side) are bound to a key combination
> instead of a generating classic "mouse" button events.
>
> The device shows up as a mouse and keyboard combination: when the middle
> button is pressed it sends a key (as keyboard) combination, the same
> happens for the two side button. The left/right/wheel work as expected
> from a mouse. To complicate things further, the middle button sends
> different keys combinations between odd and even presses.
> In the "even" press it also sends a left click. But the worst thing
> is that no event is generated when the middle button is released.
>
> It is possible to re-configure the mouse sending a command (see function
> m560_send_config_command()). After this command the mouse sends some
> useful data when the buttons are pressed and/or released.
>
> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
> ---

Works for me:

Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Thanks!
Benjamin

>  drivers/hid/hid-logitech-hidpp.c | 241 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 238 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index b3cf6fd..2275f2a 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -40,8 +40,9 @@ MODULE_PARM_DESC(disable_raw_mode,
>  #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)
> @@ -941,6 +942,221 @@ static int wtp_connect(struct hid_device *hdev, bool connected)
>                         true, true);
>  }
>
> +/*
> + * copied from hid-core.c
> + */
> +static u32 hidpp_extract(u8 *report, unsigned offset, unsigned n)
> +{
> +       u64 x;
> +
> +       report += offset >> 3;  /* adjust byte index */
> +       offset &= 7;            /* now only need bit offset into one byte */
> +       x = get_unaligned_le64(report);
> +       x = (x >> offset) & ((1ULL << n) - 1);  /* extract bit field */
> +       return (u32)x;
> +}
> +
> +/* ------------------------------------------------------------------------- */
> +/* 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 things further, 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 sends 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 {
> +       struct input_dev *input;
> +};
> +
> +/* how buttons are mapped in the report */
> +#define M560_MOUSE_BTN_LEFT            0x01
> +#define M560_MOUSE_BTN_RIGHT           0x02
> +#define M560_MOUSE_BTN_WHEEL_LEFT      0x08
> +#define M560_MOUSE_BTN_WHEEL_RIGHT     0x10
> +
> +#define M560_SUB_ID                    0x0a
> +#define M560_BUTTON_MODE_REGISTER      0x35
> +
> +static int m560_send_config_command(struct hid_device *hdev, bool connected)
> +{
> +       struct hidpp_report response;
> +       struct hidpp_device *hidpp_dev;
> +
> +       hidpp_dev = hid_get_drvdata(hdev);
> +
> +       if (!connected)
> +               return -ENODEV;
> +
> +       return 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
> +       );
> +}
> +
> +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;
> +
> +       /* sanity check */
> +       if (!mydata || !mydata->input) {
> +               hid_err(hdev, "error in parameter\n");
> +               return -EINVAL;
> +       }
> +
> +       if (size < 7) {
> +               hid_err(hdev, "error in report\n");
> +               return 0;
> +       }
> +
> +       if (data[0] == REPORT_ID_HIDPP_LONG &&
> +           data[2] == M560_SUB_ID && data[6] == 0x00) {
> +               /*
> +                * m560 mouse report for middle, forward and backward button
> +                *
> +                * data[0] = 0x11
> +                * data[1] = device-id
> +                * data[2] = 0x0a
> +                * data[5] = 0xaf -> middle
> +                *           0xb0 -> forward
> +                *           0xae -> backward
> +                *           0x00 -> release all
> +                * data[6] = 0x00
> +                */
> +
> +               switch (data[5]) {
> +               case 0xaf:
> +                       input_report_key(mydata->input, BTN_MIDDLE, 1);
> +                       break;
> +               case 0xb0:
> +                       input_report_key(mydata->input, BTN_FORWARD, 1);
> +                       break;
> +               case 0xae:
> +                       input_report_key(mydata->input, BTN_BACK, 1);
> +                       break;
> +               case 0x00:
> +                       input_report_key(mydata->input, BTN_BACK, 0);
> +                       input_report_key(mydata->input, BTN_FORWARD, 0);
> +                       input_report_key(mydata->input, BTN_MIDDLE, 0);
> +                       break;
> +               default:
> +                       hid_err(hdev, "error in report\n");
> +                       return 0;
> +               }
> +               input_sync(mydata->input);
> +
> +       } else if (data[0] == 0x02) {
> +               /*
> +                * Logitech M560 mouse report
> +                *
> +                * data[0] = type (0x02)
> +                * data[1..2] = buttons
> +                * data[3..5] = xy
> +                * data[6] = wheel
> +                */
> +
> +               int v;
> +
> +               input_report_key(mydata->input, BTN_LEFT,
> +                       !!(data[1] & M560_MOUSE_BTN_LEFT));
> +               input_report_key(mydata->input, BTN_RIGHT,
> +                       !!(data[1] & M560_MOUSE_BTN_RIGHT));
> +
> +               if (data[1] & M560_MOUSE_BTN_WHEEL_LEFT)
> +                       input_report_rel(mydata->input, REL_HWHEEL, -1);
> +               else if (data[1] & M560_MOUSE_BTN_WHEEL_RIGHT)
> +                       input_report_rel(mydata->input, REL_HWHEEL, 1);
> +
> +               v = hid_snto32(hidpp_extract(data+3, 0, 12), 12);
> +               input_report_rel(mydata->input, REL_X, v);
> +
> +               v = hid_snto32(hidpp_extract(data+3, 12, 12), 12);
> +               input_report_rel(mydata->input, REL_Y, v);
> +
> +               v = hid_snto32(data[6], 8);
> +               input_report_rel(mydata->input, REL_WHEEL, v);
> +
> +               input_sync(mydata->input);
> +       }
> +
> +       return 1;
> +}
> +
> +static void m560_populate_input(struct hidpp_device *hidpp,
> +               struct input_dev *input_dev, bool origin_is_hid_core)
> +{
> +       struct m560_private_data *mydata = hidpp->private_data;
> +
> +       mydata->input = input_dev;
> +
> +       __set_bit(EV_KEY, mydata->input->evbit);
> +       __set_bit(BTN_MIDDLE, mydata->input->keybit);
> +       __set_bit(BTN_RIGHT, mydata->input->keybit);
> +       __set_bit(BTN_LEFT, mydata->input->keybit);
> +       __set_bit(BTN_BACK, mydata->input->keybit);
> +       __set_bit(BTN_FORWARD, mydata->input->keybit);
> +
> +       __set_bit(EV_REL, mydata->input->evbit);
> +       __set_bit(REL_X, mydata->input->relbit);
> +       __set_bit(REL_Y, mydata->input->relbit);
> +       __set_bit(REL_WHEEL, mydata->input->relbit);
> +       __set_bit(REL_HWHEEL, mydata->input->relbit);
> +}
> +
> +static int m560_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> +               struct hid_field *field, struct hid_usage *usage,
> +               unsigned long **bit, int *max)
> +{
> +       return -1;
> +}
> +
>  /* -------------------------------------------------------------------------- */
>  /* Generic HID++ devices                                                      */
>  /* -------------------------------------------------------------------------- */
> @@ -953,6 +1169,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 m560_input_mapping(hdev, hi, field, usage, bit, max);
>
>         return 0;
>  }
> @@ -962,6 +1181,8 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
>  {
>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>                 wtp_populate_input(hidpp, input, origin_is_hid_core);
> +       else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
> +               m560_populate_input(hidpp, input, origin_is_hid_core);
>  }
>
>  static void hidpp_input_configured(struct hid_device *hdev,
> @@ -1049,6 +1270,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(hdev, data, size);
>
>         return 0;
>  }
> @@ -1126,6 +1349,10 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
>                 ret = wtp_connect(hdev, connected);
>                 if (ret)
>                         return;
> +       } else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) {
> +               ret = m560_send_config_command(hdev, connected);
> +               if (ret)
> +                       return;
>         }
>
>         if (!connected || hidpp->delayed_input)
> @@ -1201,7 +1428,11 @@ 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;
> +       } else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) {
> +               ret = m560_allocate(hdev);
> +               if (ret)
> +                       goto allocate_fail;
>         }
>
>         INIT_WORK(&hidpp->work, delayed_work_cb);
> @@ -1268,7 +1499,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;
>  }
> @@ -1301,6 +1532,10 @@ 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_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
>
>         { HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
>                 USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
> --
> 2.1.4
>

[PATCH] Add driver for mouse logitech M560

[Goffredo Baroncelli]

From: Goffredo Baroncelli <kreijack@inwind.it>

The Logitech M560 is a wireless mouse designed for windows 8 which uses
the unifying receiver.
Compared to a standard one, some buttons (the middle one and the
two ones placed on the side) are bound to a key combination
instead of a generating classic "mouse" button events.

The device shows up as a mouse and keyboard combination: when the middle
button is pressed it sends a key (as keyboard) combination, the same
happens for the two side button. The left/right/wheel work as expected
from a mouse. To complicate things further, the middle button sends
different keys combinations between odd and even presses.
In the "even" press it also sends a left click. But the worst thing
is that no event is generated when the middle button is released.

It is possible to re-configure the mouse sending a command (see function
m560_send_config_command()). After this command the mouse sends some
useful data when the buttons are pressed and/or released.

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 drivers/hid/hid-logitech-hidpp.c | 241 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 238 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index b3cf6fd..2275f2a 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -40,8 +40,9 @@ MODULE_PARM_DESC(disable_raw_mode,
 #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)
@@ -941,6 +942,221 @@ static int wtp_connect(struct hid_device *hdev, bool connected)
 			true, true);
 }
 
+/*
+ * copied from hid-core.c
+ */
+static u32 hidpp_extract(u8 *report, unsigned offset, unsigned n)
+{
+	u64 x;
+
+	report += offset >> 3;  /* adjust byte index */
+	offset &= 7;            /* now only need bit offset into one byte */
+	x = get_unaligned_le64(report);
+	x = (x >> offset) & ((1ULL << n) - 1);  /* extract bit field */
+	return (u32)x;
+}
+
+/* ------------------------------------------------------------------------- */
+/* 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 things further, 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 sends 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 {
+	struct input_dev *input;
+};
+
+/* how buttons are mapped in the report */
+#define M560_MOUSE_BTN_LEFT		0x01
+#define M560_MOUSE_BTN_RIGHT		0x02
+#define M560_MOUSE_BTN_WHEEL_LEFT	0x08
+#define M560_MOUSE_BTN_WHEEL_RIGHT	0x10
+
+#define M560_SUB_ID			0x0a
+#define M560_BUTTON_MODE_REGISTER	0x35
+
+static int m560_send_config_command(struct hid_device *hdev, bool connected)
+{
+	struct hidpp_report response;
+	struct hidpp_device *hidpp_dev;
+
+	hidpp_dev = hid_get_drvdata(hdev);
+
+	if (!connected)
+		return -ENODEV;
+
+	return 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
+	);
+}
+
+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;
+
+	/* sanity check */
+	if (!mydata || !mydata->input) {
+		hid_err(hdev, "error in parameter\n");
+		return -EINVAL;
+	}
+
+	if (size < 7) {
+		hid_err(hdev, "error in report\n");
+		return 0;
+	}
+
+	if (data[0] == REPORT_ID_HIDPP_LONG &&
+	    data[2] == M560_SUB_ID && data[6] == 0x00) {
+		/*
+		 * m560 mouse report for middle, forward and backward button
+		 *
+		 * data[0] = 0x11
+		 * data[1] = device-id
+		 * data[2] = 0x0a
+		 * data[5] = 0xaf -> middle
+		 *	     0xb0 -> forward
+		 *	     0xae -> backward
+		 *	     0x00 -> release all
+		 * data[6] = 0x00
+		 */
+
+		switch (data[5]) {
+		case 0xaf:
+			input_report_key(mydata->input, BTN_MIDDLE, 1);
+			break;
+		case 0xb0:
+			input_report_key(mydata->input, BTN_FORWARD, 1);
+			break;
+		case 0xae:
+			input_report_key(mydata->input, BTN_BACK, 1);
+			break;
+		case 0x00:
+			input_report_key(mydata->input, BTN_BACK, 0);
+			input_report_key(mydata->input, BTN_FORWARD, 0);
+			input_report_key(mydata->input, BTN_MIDDLE, 0);
+			break;
+		default:
+			hid_err(hdev, "error in report\n");
+			return 0;
+		}
+		input_sync(mydata->input);
+
+	} else if (data[0] == 0x02) {
+		/*
+		 * Logitech M560 mouse report
+		 *
+		 * data[0] = type (0x02)
+		 * data[1..2] = buttons
+		 * data[3..5] = xy
+		 * data[6] = wheel
+		 */
+
+		int v;
+
+		input_report_key(mydata->input, BTN_LEFT,
+			!!(data[1] & M560_MOUSE_BTN_LEFT));
+		input_report_key(mydata->input, BTN_RIGHT,
+			!!(data[1] & M560_MOUSE_BTN_RIGHT));
+
+		if (data[1] & M560_MOUSE_BTN_WHEEL_LEFT)
+			input_report_rel(mydata->input, REL_HWHEEL, -1);
+		else if (data[1] & M560_MOUSE_BTN_WHEEL_RIGHT)
+			input_report_rel(mydata->input, REL_HWHEEL, 1);
+
+		v = hid_snto32(hidpp_extract(data+3, 0, 12), 12);
+		input_report_rel(mydata->input, REL_X, v);
+
+		v = hid_snto32(hidpp_extract(data+3, 12, 12), 12);
+		input_report_rel(mydata->input, REL_Y, v);
+
+		v = hid_snto32(data[6], 8);
+		input_report_rel(mydata->input, REL_WHEEL, v);
+
+		input_sync(mydata->input);
+	}
+
+	return 1;
+}
+
+static void m560_populate_input(struct hidpp_device *hidpp,
+		struct input_dev *input_dev, bool origin_is_hid_core)
+{
+	struct m560_private_data *mydata = hidpp->private_data;
+
+	mydata->input = input_dev;
+
+	__set_bit(EV_KEY, mydata->input->evbit);
+	__set_bit(BTN_MIDDLE, mydata->input->keybit);
+	__set_bit(BTN_RIGHT, mydata->input->keybit);
+	__set_bit(BTN_LEFT, mydata->input->keybit);
+	__set_bit(BTN_BACK, mydata->input->keybit);
+	__set_bit(BTN_FORWARD, mydata->input->keybit);
+
+	__set_bit(EV_REL, mydata->input->evbit);
+	__set_bit(REL_X, mydata->input->relbit);
+	__set_bit(REL_Y, mydata->input->relbit);
+	__set_bit(REL_WHEEL, mydata->input->relbit);
+	__set_bit(REL_HWHEEL, mydata->input->relbit);
+}
+
+static int m560_input_mapping(struct hid_device *hdev, struct hid_input *hi,
+		struct hid_field *field, struct hid_usage *usage,
+		unsigned long **bit, int *max)
+{
+	return -1;
+}
+
 /* -------------------------------------------------------------------------- */
 /* Generic HID++ devices                                                      */
 /* -------------------------------------------------------------------------- */
@@ -953,6 +1169,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 m560_input_mapping(hdev, hi, field, usage, bit, max);
 
 	return 0;
 }
@@ -962,6 +1181,8 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
 {
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		wtp_populate_input(hidpp, input, origin_is_hid_core);
+	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
+		m560_populate_input(hidpp, input, origin_is_hid_core);
 }
 
 static void hidpp_input_configured(struct hid_device *hdev,
@@ -1049,6 +1270,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(hdev, data, size);
 
 	return 0;
 }
@@ -1126,6 +1349,10 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 		ret = wtp_connect(hdev, connected);
 		if (ret)
 			return;
+	} else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) {
+		ret = m560_send_config_command(hdev, connected);
+		if (ret)
+			return;
 	}
 
 	if (!connected || hidpp->delayed_input)
@@ -1201,7 +1428,11 @@ 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;
+	} else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) {
+		ret = m560_allocate(hdev);
+		if (ret)
+			goto allocate_fail;
 	}
 
 	INIT_WORK(&hidpp->work, delayed_work_cb);
@@ -1268,7 +1499,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;
 }
@@ -1301,6 +1532,10 @@ 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_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
 
 	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
-- 
2.1.4

Re: [PATCH] Add driver for mouse logitech M560

[Antonio Ospite]

On Wed, 29 Apr 2015 20:24:14 +0200
Goffredo Baroncelli <kreijack@libero.it> wrote:

> From: Goffredo Baroncelli <kreijack@inwind.it>
>

Thanks Goffredo, the new version looks more readable indeed.

I am adding some comments below.

I am commenting also trivial stuff which normally wouldn't be worth
mentioning, but since it's your first big kernel patch and it looks like
you are going to send a v4 anyway...

> The Logitech M560 is a wireless mouse designed for windows 8 which uses
> the unifying receiver.
> Compared to a standard one, some buttons (the middle one and the
> two ones placed on the side) are bound to a key combination
> instead of a classic "mouse" button.

I'd say something like:
	... instead of generating classic "mouse" button events.

> 
> The device shows up as a mouse and keyboard combination: when the middle
> button is pressed it sends a key (as keyboard) combination, the same happens

scripts/checkpatch.pl gives a warning on the line above, to fix this
wrap the lines at 72 or max 75 characters in git commit messages.

> for the two side button. The left/right/wheel work  as expected from a
                                                    ^^
Two consecutive spaces in the above line.

> mouse. To complicate things further, the middle button sends different keys
> combinations between odd and even presses.
> In the "even" press it also sends a left click. But the worst thing
> is that no event is generated when the middle button is released.
> 
> It is possible to re-configure the mouse sending a command (see function
> m560_send_config_command()). After this command the mouse sends some
> useful data when the buttons are pressed and/or released

Missing period at the end of the paragraph.

> 
> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
> ---
>  drivers/hid/hid-logitech-hidpp.c | 243 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 240 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index b3cf6fd..d7e33c8 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -40,8 +40,9 @@ MODULE_PARM_DESC(disable_raw_mode,
>  #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)
> @@ -942,6 +943,223 @@ static int wtp_connect(struct hid_device *hdev, bool connected)
>  }
>  
>  /* -------------------------------------------------------------------------- */
> +/* Logitech M560 devices                                                     */

Align the end of comment in the line above with the others.

> +/* -------------------------------------------------------------------------- */
> +
> +/*
> + * 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

Say "To complicate things further" just like in the commit message.

> + * 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 {
> +	struct input_dev *input;
> +};
> +
> +/* 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

You could strip all these comments before functions, the function names
and signatures are quite self-explanatory.

Anyway, if you want to keep them, for the @return line use something
like:
  * @return: 0 on success or error code on failure

> + */
> +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
> +	);

If you are not going use ret or the response variable after the function
call you could just:

	return hidpp_send_rap_command_sync(

> +
> +	return ret;
> +}
> +
> +/*
> + * m560_allocate - allocate the M560 mouse private data, and link it
> + *                 to the hidpp_device struct
> + *
> + * @hdev: hidpp device where the mouse belongs
> + *
> + * @return: 0 OK
> + */
> +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;
> +};
> +
> +/*
> + * m560_raw_event - process raw event
> + */
> +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;
> +
> +	/* check if the data is a mouse related report */

This checks if the data is NOT a mouse related report, right?

Or does the comment refer to all the checks below, not just to the one
following it?

> +	if (data[0] != 0x02 && data[2] != M560_SUB_ID)
> +		return 0;
> +
> +	if (data[0] == REPORT_ID_HIDPP_LONG &&
> +	    data[2] == M560_SUB_ID && data[06] == 0x00) {
                                           ^^
Octal value 06 used as array index.

> +		/*
> +		 * m560 mouse button report

Specify that these are _extra_ mouse buttons, or that this is a "buttons
only" report. because in the "else if" branch we are handling some
buttons too.

> +		 *
> +		 * data[0] = 0x11
> +		 * data[1] = deviceid
> +		 * data[2] = 0x0a
> +		 * data[5] = button (0xaf->middle, 0xb0->forward,
> +		 *                   0xaf ->backward, 0x00->release all)

Tabulate the values like that:

		 * data[5] = button (0xaf: middle button,
		 *                   0xb0: forward button,
		 *                   0xaf: backward button,
		 *                   0x00: release all)

the -> symbol makes me think to the operator, but it could be just me.

> +		 * data[6] = 0x00
> +		 */
> +
> +		int btn;
> +
> +		/* check if the event is a button */
> +		btn = data[5];
> +		if (btn != 0x00 && btn != 0xb0 && btn != 0xae && btn != 0xaf)
> +			return 1;
> +
> +		if (btn == 0xaf) {
> +			input_report_key(mydata->input, BTN_MIDDLE, 1);
> +		} else if (btn == 0xb0) {
> +			input_report_key(mydata->input, BTN_FORWARD, 1);
> +		} else if (btn == 0xae) {
> +			input_report_key(mydata->input, BTN_BACK, 1);
> +		} else if (btn == 0x00) {
> +			input_report_key(mydata->input, BTN_BACK, 0);
> +			input_report_key(mydata->input, BTN_FORWARD, 0);
> +			input_report_key(mydata->input, BTN_MIDDLE, 0);
> +		}

A switch statement could look prettier here, and the "NOT a button"
check from above can be the default case.

> +
> +		input_sync(mydata->input);
> +
> +	} else if (data[0] == 0x02) {
> +		/*
> +		 * Logitech M560 mouse report
> +		 *
> +		 * data[0] = type (0x02)
> +		 * data[1..2] = buttons
> +		 * data[3..5] = xy
> +		 * data[6] = wheel
> +		 */
> +
> +		int v;
> +		int btn = data[1];
> +
> +		input_report_key(mydata->input, BTN_LEFT,
> +			!!(btn & M560_MOUSE_BTN_LEFT));
> +		input_report_key(mydata->input, BTN_RIGHT,
> +			!!(btn & M560_MOUSE_BTN_RIGHT));
> +
> +		if (btn & M560_MOUSE_BTN_WHEEL_LEFT)
> +			input_report_rel(mydata->input, REL_HWHEEL, -1);
> +		else if (btn & M560_MOUSE_BTN_WHEEL_RIGHT)
> +			input_report_rel(mydata->input, REL_HWHEEL, 1);
> +
> +		v = (int)data[3] | (int)((data[4] & 0x0f) << 8);
> +		if (v & (1<<11))
> +			v = ~0xfff | v;
> +		input_report_rel(mydata->input, REL_X, v);
> +		v = (int)(data[4] >> 4) | (int)(data[5] << 4);
> +		if (v & (1<<11))
> +			v = ~0xfff | v;
> +		input_report_rel(mydata->input, REL_Y, v);
> +		v = (int)data[6];
> +		if (v & (1<<7))
> +			v = ~0xff | v;
> +		input_report_rel(mydata->input, REL_WHEEL, v);
> +
> +		input_sync(mydata->input);
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * config the input device
> + */
> +static void m560_populate_input(struct hidpp_device *hidpp,
> +		struct input_dev *input_dev, bool origin_is_hid_core)
> +{
> +	struct m560_private_data *mydata = hidpp->private_data;
> +
> +	if ((hidpp->quirks & HIDPP_QUIRK_MULTI_INPUT) && origin_is_hid_core)
> +		/* this is the generic hid-input call */
> +		return;
> +
> +	mydata->input = input_dev;
> +
> +	__set_bit(EV_KEY, mydata->input->evbit);
> +	__set_bit(BTN_MIDDLE, mydata->input->keybit);
> +	__set_bit(BTN_RIGHT, mydata->input->keybit);
> +	__set_bit(BTN_LEFT, mydata->input->keybit);
> +	__set_bit(BTN_BACK, mydata->input->keybit);
> +	__set_bit(BTN_FORWARD, mydata->input->keybit);
> +
> +	__set_bit(EV_REL, mydata->input->evbit);
> +	__set_bit(REL_X, mydata->input->relbit);
> +	__set_bit(REL_Y, mydata->input->relbit);
> +	__set_bit(REL_WHEEL, mydata->input->relbit);
> +	__set_bit(REL_HWHEEL, mydata->input->relbit);
> +
> +}
> +
> +/* -------------------------------------------------------------------------- */
>  /* Generic HID++ devices                                                      */
>  /* -------------------------------------------------------------------------- */
>  
> @@ -953,6 +1171,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;
>  }
> @@ -962,6 +1183,8 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
>  {
>  	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>  		wtp_populate_input(hidpp, input, origin_is_hid_core);
> +	if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
> +		m560_populate_input(hidpp, input, origin_is_hid_core);
>  }
>  
>  static void hidpp_input_configured(struct hid_device *hdev,
> @@ -1049,6 +1272,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;
>  }
> @@ -1126,6 +1351,8 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
>  		ret = wtp_connect(hdev, connected);
>  		if (ret)
>  			return;
> +	} else if ((hidpp->quirks & HIDPP_QUIRK_CLASS_M560) && connected) {
> +		m560_send_config_command(hdev);

Handle the return value here. Again, for symmetry with the code above.

To make the two conditionals look even more alike you could even handle
the "connected" check in m560_send_config_command(), returning a non
zero value (e.g. -ENODEV) there if !connected; value which you will
check here.

>  	}
>  
>  	if (!connected || hidpp->delayed_input)
> @@ -1201,7 +1428,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 please, this way it's super clear that the two allocate
calls are exclusive.

> +		ret = m560_allocate(hdev);
> +		if (ret)
> +			goto allocate_fail;
>  	}
>  
>  	INIT_WORK(&hidpp->work, delayed_work_cb);
> @@ -1268,7 +1500,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;
>  }
> @@ -1301,6 +1533,11 @@ 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_DELAYED_INIT  |
> +			 HIDPP_QUIRK_CLASS_M560 },

Indent like for the other devices, the two quirks fit on the same line.

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


-- 
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] Add driver for mouse logitech M560

[Benjamin Tissoires]

Hi Goffredo,

On Wed, Apr 29, 2015 at 2:24 PM, Goffredo Baroncelli <kreijack@libero.it> wrote:
> From: Goffredo Baroncelli <kreijack@inwind.it>
>
> The Logitech M560 is a wireless mouse designed for windows 8 which uses
> the unifying receiver.
> Compared to a standard one, some buttons (the middle one and the
> two ones placed on the side) are bound to a key combination
> instead of a classic "mouse" button.
>
> The device shows up as a mouse and keyboard combination: when the middle
> button is pressed it sends a key (as keyboard) combination, the same happens
> for the two side button. The left/right/wheel work  as expected from a
> mouse. To complicate things further, the middle button sends different keys
> combinations between odd and even presses.
> In the "even" press it also sends a left click. But the worst thing
> is that no event is generated when the middle button is released.
>
> It is possible to re-configure the mouse sending a command (see function
> m560_send_config_command()). After this command the mouse sends some
> useful data when the buttons are pressed and/or released
>
> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
> ---

Thanks for this new version. I have a few nitpicks which should not take too long to apply:

>  drivers/hid/hid-logitech-hidpp.c | 243 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 240 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index b3cf6fd..d7e33c8 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -40,8 +40,9 @@ MODULE_PARM_DESC(disable_raw_mode,
>  #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)
> @@ -942,6 +943,223 @@ static int 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 {
> +       struct input_dev *input;
> +};
> +
> +/* 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;
> +}
> +
> +/*
> + * m560_allocate - allocate the M560 mouse private data, and link it
> + *                 to the hidpp_device struct
> + *
> + * @hdev: hidpp device where the mouse belongs
> + *
> + * @return: 0 OK
> + */
> +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;
> +};
> +
> +/*
> + * m560_raw_event - process raw event
> + */
> +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;
> +
> +       /* check if the data is a mouse related report */
> +       if (data[0] != 0x02 && data[2] != M560_SUB_ID)
> +               return 0;
> +
> +       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;
> +
> +               /* check if the event is a button */
> +               btn = data[5];
> +               if (btn != 0x00 && btn != 0xb0 && btn != 0xae && btn != 0xaf)
> +                       return 1;
> +
> +               if (btn == 0xaf) {
> +                       input_report_key(mydata->input, BTN_MIDDLE, 1);
> +               } else if (btn == 0xb0) {
> +                       input_report_key(mydata->input, BTN_FORWARD, 1);
> +               } else if (btn == 0xae) {
> +                       input_report_key(mydata->input, BTN_BACK, 1);
> +               } else if (btn == 0x00) {
> +                       input_report_key(mydata->input, BTN_BACK, 0);
> +                       input_report_key(mydata->input, BTN_FORWARD, 0);
> +                       input_report_key(mydata->input, BTN_MIDDLE, 0);
> +               }
> +
> +               input_sync(mydata->input);
> +
> +       } else if (data[0] == 0x02) {
> +               /*
> +                * Logitech M560 mouse report
> +                *
> +                * data[0] = type (0x02)
> +                * data[1..2] = buttons
> +                * data[3..5] = xy
> +                * data[6] = wheel
> +                */
> +
> +               int v;
> +               int btn = data[1];
> +
> +               input_report_key(mydata->input, BTN_LEFT,
> +                       !!(btn & M560_MOUSE_BTN_LEFT));
> +               input_report_key(mydata->input, BTN_RIGHT,
> +                       !!(btn & M560_MOUSE_BTN_RIGHT));
> +
> +               if (btn & M560_MOUSE_BTN_WHEEL_LEFT)
> +                       input_report_rel(mydata->input, REL_HWHEEL, -1);
> +               else if (btn & M560_MOUSE_BTN_WHEEL_RIGHT)
> +                       input_report_rel(mydata->input, REL_HWHEEL, 1);
> +
> +               v = (int)data[3] | (int)((data[4] & 0x0f) << 8);
> +               if (v & (1<<11))
> +                       v = ~0xfff | v;
> +               input_report_rel(mydata->input, REL_X, v);
> +               v = (int)(data[4] >> 4) | (int)(data[5] << 4);
> +               if (v & (1<<11))
> +                       v = ~0xfff | v;
> +               input_report_rel(mydata->input, REL_Y, v);
> +               v = (int)data[6];
> +               if (v & (1<<7))
> +                       v = ~0xff | v;
> +               input_report_rel(mydata->input, REL_WHEEL, v);

This looks messy to me.

How about you copy the extract() function from hid-core here as such (note
the hid_snto32 at the end which is not in the original function):

/* duplicated from hid-core.c */
static int hidpp_extract(struct hid_device *hdev, u8 *report, unsigned offset,
		unsigned n)
{
	u64 x;

	if (n > 32)
		hid_warn(hdev, "hidpp_extract() called with n (%d) > 32!\n", n);

	report += offset >> 3;  /* adjust byte index */
	offset &= 7;            /* now only need bit offset into one byte */
	x = get_unaligned_le64(report);
	x = (x >> offset) & ((1ULL << n) - 1);  /* extract bit field */
	return hid_snto32(x, n);
}

and then you can have:
x = hidpp_extract(hdev, &data[3], 0, 12);
y = hidpp_extract(hdev, &data[3], 12, 12);
wheel = hid_snto32(data[6], 8);

> +
> +               input_sync(mydata->input);
> +       }
> +
> +       return 0;
> +}
> +
> +/*
> + * config the input device
> + */
> +static void m560_populate_input(struct hidpp_device *hidpp,
> +               struct input_dev *input_dev, bool origin_is_hid_core)
> +{
> +       struct m560_private_data *mydata = hidpp->private_data;
> +
> +       if ((hidpp->quirks & HIDPP_QUIRK_MULTI_INPUT) && origin_is_hid_core)

Do you really intend to use HIDPP_QUIRK_MULTI_INPUT with this particular mouse?
If not, we can drop the test.

> +               /* this is the generic hid-input call */
> +               return;
> +
> +       mydata->input = input_dev;
> +
> +       __set_bit(EV_KEY, mydata->input->evbit);
> +       __set_bit(BTN_MIDDLE, mydata->input->keybit);
> +       __set_bit(BTN_RIGHT, mydata->input->keybit);
> +       __set_bit(BTN_LEFT, mydata->input->keybit);
> +       __set_bit(BTN_BACK, mydata->input->keybit);
> +       __set_bit(BTN_FORWARD, mydata->input->keybit);
> +
> +       __set_bit(EV_REL, mydata->input->evbit);
> +       __set_bit(REL_X, mydata->input->relbit);
> +       __set_bit(REL_Y, mydata->input->relbit);
> +       __set_bit(REL_WHEEL, mydata->input->relbit);
> +       __set_bit(REL_HWHEEL, mydata->input->relbit);
> +
> +}
> +
> +/* -------------------------------------------------------------------------- */
>  /* Generic HID++ devices                                                      */
>  /* -------------------------------------------------------------------------- */
>
> @@ -953,6 +1171,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;

I know it will be a one line function, but I'd rather having a separate
m560_input_mapping() function for that. Otherwise we may end up adding other
generic behavior below and it will interfere with the current code.

>
>         return 0;
>  }
> @@ -962,6 +1183,8 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
>  {
>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
>                 wtp_populate_input(hidpp, input, origin_is_hid_core);
> +       if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)

else if

> +               m560_populate_input(hidpp, input, origin_is_hid_core);
>  }
>
>  static void hidpp_input_configured(struct hid_device *hdev,
> @@ -1049,6 +1272,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;
>  }
> @@ -1126,6 +1351,8 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
>                 ret = wtp_connect(hdev, connected);
>                 if (ret)
>                         return;
> +       } else if ((hidpp->quirks & HIDPP_QUIRK_CLASS_M560) && connected) {
> +               m560_send_config_command(hdev);
>         }
>
>         if (!connected || hidpp->delayed_input)
> @@ -1201,7 +1428,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);
> @@ -1268,7 +1500,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;
>  }
> @@ -1301,6 +1533,11 @@ 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_DELAYED_INIT  |
> +                        HIDPP_QUIRK_CLASS_M560 },
>
>         { HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
>                 USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
> --
> 2.1.4
>

The rest looks good to me.

Thanks for the hard work!

Cheers,
Benjamin

[PATCH] Add driver for mouse logitech M560

[Goffredo Baroncelli]

From: Goffredo Baroncelli <kreijack@inwind.it>

The Logitech M560 is a wireless mouse designed for windows 8 which uses
the unifying receiver.
Compared to a standard one, some buttons (the middle one and the
two ones placed on the side) are bound to a key combination
instead of a classic "mouse" button.

The device shows up as a mouse and keyboard combination: when the middle
button is pressed it sends a key (as keyboard) combination, the same happens
for the two side button. The left/right/wheel work  as expected from a
mouse. To complicate things further, the middle button sends different keys
combinations between odd and even presses.
In the "even" press it also sends a left click. But the worst thing
is that no event is generated when the middle button is released.

It is possible to re-configure the mouse sending a command (see function
m560_send_config_command()). After this command the mouse sends some
useful data when the buttons are pressed and/or released

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 drivers/hid/hid-logitech-hidpp.c | 243 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 240 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index b3cf6fd..d7e33c8 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -40,8 +40,9 @@ MODULE_PARM_DESC(disable_raw_mode,
 #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)
@@ -942,6 +943,223 @@ static int 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 {
+	struct input_dev *input;
+};
+
+/* 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;
+}
+
+/*
+ * m560_allocate - allocate the M560 mouse private data, and link it
+ *                 to the hidpp_device struct
+ *
+ * @hdev: hidpp device where the mouse belongs
+ *
+ * @return: 0 OK
+ */
+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;
+};
+
+/*
+ * m560_raw_event - process raw event
+ */
+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;
+
+	/* check if the data is a mouse related report */
+	if (data[0] != 0x02 && data[2] != M560_SUB_ID)
+		return 0;
+
+	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;
+
+		/* check if the event is a button */
+		btn = data[5];
+		if (btn != 0x00 && btn != 0xb0 && btn != 0xae && btn != 0xaf)
+			return 1;
+
+		if (btn == 0xaf) {
+			input_report_key(mydata->input, BTN_MIDDLE, 1);
+		} else if (btn == 0xb0) {
+			input_report_key(mydata->input, BTN_FORWARD, 1);
+		} else if (btn == 0xae) {
+			input_report_key(mydata->input, BTN_BACK, 1);
+		} else if (btn == 0x00) {
+			input_report_key(mydata->input, BTN_BACK, 0);
+			input_report_key(mydata->input, BTN_FORWARD, 0);
+			input_report_key(mydata->input, BTN_MIDDLE, 0);
+		}
+
+		input_sync(mydata->input);
+
+	} else if (data[0] == 0x02) {
+		/*
+		 * Logitech M560 mouse report
+		 *
+		 * data[0] = type (0x02)
+		 * data[1..2] = buttons
+		 * data[3..5] = xy
+		 * data[6] = wheel
+		 */
+
+		int v;
+		int btn = data[1];
+
+		input_report_key(mydata->input, BTN_LEFT,
+			!!(btn & M560_MOUSE_BTN_LEFT));
+		input_report_key(mydata->input, BTN_RIGHT,
+			!!(btn & M560_MOUSE_BTN_RIGHT));
+
+		if (btn & M560_MOUSE_BTN_WHEEL_LEFT)
+			input_report_rel(mydata->input, REL_HWHEEL, -1);
+		else if (btn & M560_MOUSE_BTN_WHEEL_RIGHT)
+			input_report_rel(mydata->input, REL_HWHEEL, 1);
+
+		v = (int)data[3] | (int)((data[4] & 0x0f) << 8);
+		if (v & (1<<11))
+			v = ~0xfff | v;
+		input_report_rel(mydata->input, REL_X, v);
+		v = (int)(data[4] >> 4) | (int)(data[5] << 4);
+		if (v & (1<<11))
+			v = ~0xfff | v;
+		input_report_rel(mydata->input, REL_Y, v);
+		v = (int)data[6];
+		if (v & (1<<7))
+			v = ~0xff | v;
+		input_report_rel(mydata->input, REL_WHEEL, v);
+
+		input_sync(mydata->input);
+	}
+
+	return 0;
+}
+
+/*
+ * config the input device
+ */
+static void m560_populate_input(struct hidpp_device *hidpp,
+		struct input_dev *input_dev, bool origin_is_hid_core)
+{
+	struct m560_private_data *mydata = hidpp->private_data;
+
+	if ((hidpp->quirks & HIDPP_QUIRK_MULTI_INPUT) && origin_is_hid_core)
+		/* this is the generic hid-input call */
+		return;
+
+	mydata->input = input_dev;
+
+	__set_bit(EV_KEY, mydata->input->evbit);
+	__set_bit(BTN_MIDDLE, mydata->input->keybit);
+	__set_bit(BTN_RIGHT, mydata->input->keybit);
+	__set_bit(BTN_LEFT, mydata->input->keybit);
+	__set_bit(BTN_BACK, mydata->input->keybit);
+	__set_bit(BTN_FORWARD, mydata->input->keybit);
+
+	__set_bit(EV_REL, mydata->input->evbit);
+	__set_bit(REL_X, mydata->input->relbit);
+	__set_bit(REL_Y, mydata->input->relbit);
+	__set_bit(REL_WHEEL, mydata->input->relbit);
+	__set_bit(REL_HWHEEL, mydata->input->relbit);
+
+}
+
+/* -------------------------------------------------------------------------- */
 /* Generic HID++ devices                                                      */
 /* -------------------------------------------------------------------------- */
 
@@ -953,6 +1171,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;
 }
@@ -962,6 +1183,8 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
 {
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		wtp_populate_input(hidpp, input, origin_is_hid_core);
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
+		m560_populate_input(hidpp, input, origin_is_hid_core);
 }
 
 static void hidpp_input_configured(struct hid_device *hdev,
@@ -1049,6 +1272,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;
 }
@@ -1126,6 +1351,8 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 		ret = wtp_connect(hdev, connected);
 		if (ret)
 			return;
+	} else if ((hidpp->quirks & HIDPP_QUIRK_CLASS_M560) && connected) {
+		m560_send_config_command(hdev);
 	}
 
 	if (!connected || hidpp->delayed_input)
@@ -1201,7 +1428,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);
@@ -1268,7 +1500,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;
 }
@@ -1301,6 +1533,11 @@ 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_DELAYED_INIT  |
+			 HIDPP_QUIRK_CLASS_M560 },
 
 	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
-- 
2.1.4

[PATCH] Add driver for mouse logitech M560

[Goffredo Baroncelli]

From: Goffredo Baroncelli <kreijack@inwind.it>

The Logitech M560 is a wireless mouse designed for windows 8 which uses
the unifying receiver.
Compared to a standard one, some buttons (the middle one and the
two ones placed on the side) are bound to a key combination
instead of a classic "mouse" button.

The device shows up as a mouse and keyboard combination: when the middle
button is pressed it sends a key (as keyboard) combination, the same happens
for the two side button. The left/right/wheel work  as expected from a
mouse. To complicate things further, the middle button sends different keys
combinations between odd and even presses.
In the "even" press it also sends a left click. But the worst thing
is that no event is generated when the middle button is released.

It is possible to re-configure the mouse sending a command (see function
m560_send_config_command()). After this command the mouse sends some
useful data when the buttons are pressed and/or released

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 drivers/hid/hid-logitech-hidpp.c | 243 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 240 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index b3cf6fd..d7e33c8 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -40,8 +40,9 @@ MODULE_PARM_DESC(disable_raw_mode,
 #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)
@@ -942,6 +943,223 @@ static int 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 {
+	struct input_dev *input;
+};
+
+/* 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;
+}
+
+/*
+ * m560_allocate - allocate the M560 mouse private data, and link it
+ *                 to the hidpp_device struct
+ *
+ * @hdev: hidpp device where the mouse belongs
+ *
+ * @return: 0 OK
+ */
+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;
+};
+
+/*
+ * m560_raw_event - process raw event
+ */
+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;
+
+	/* check if the data is a mouse related report */
+	if (data[0] != 0x02 && data[2] != M560_SUB_ID)
+		return 0;
+
+	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;
+
+		/* check if the event is a button */
+		btn = data[5];
+		if (btn != 0x00 && btn != 0xb0 && btn != 0xae && btn != 0xaf)
+			return 1;
+
+		if (btn == 0xaf) {
+			input_report_key(mydata->input, BTN_MIDDLE, 1);
+		} else if (btn == 0xb0) {
+			input_report_key(mydata->input, BTN_FORWARD, 1);
+		} else if (btn == 0xae) {
+			input_report_key(mydata->input, BTN_BACK, 1);
+		} else if (btn == 0x00) {
+			input_report_key(mydata->input, BTN_BACK, 0);
+			input_report_key(mydata->input, BTN_FORWARD, 0);
+			input_report_key(mydata->input, BTN_MIDDLE, 0);
+		}
+
+		input_sync(mydata->input);
+
+	} else if (data[0] == 0x02) {
+		/*
+		 * Logitech M560 mouse report
+		 *
+		 * data[0] = type (0x02)
+		 * data[1..2] = buttons
+		 * data[3..5] = xy
+		 * data[6] = wheel
+		 */
+
+		int v;
+		int btn = data[1];
+
+		input_report_key(mydata->input, BTN_LEFT,
+			!!(btn & M560_MOUSE_BTN_LEFT));
+		input_report_key(mydata->input, BTN_RIGHT,
+			!!(btn & M560_MOUSE_BTN_RIGHT));
+
+		if (btn & M560_MOUSE_BTN_WHEEL_LEFT)
+			input_report_rel(mydata->input, REL_HWHEEL, -1);
+		else if (btn & M560_MOUSE_BTN_WHEEL_RIGHT)
+			input_report_rel(mydata->input, REL_HWHEEL, 1);
+
+		v = (int)data[3] | (int)((data[4] & 0x0f) << 8);
+		if (v & (1<<11))
+			v = ~0xfff | v;
+		input_report_rel(mydata->input, REL_X, v);
+		v = (int)(data[4] >> 4) | (int)(data[5] << 4);
+		if (v & (1<<11))
+			v = ~0xfff | v;
+		input_report_rel(mydata->input, REL_Y, v);
+		v = (int)data[6];
+		if (v & (1<<7))
+			v = ~0xff | v;
+		input_report_rel(mydata->input, REL_WHEEL, v);
+
+		input_sync(mydata->input);
+	}
+
+	return 0;
+}
+
+/*
+ * config the input device
+ */
+static void m560_populate_input(struct hidpp_device *hidpp,
+		struct input_dev *input_dev, bool origin_is_hid_core)
+{
+	struct m560_private_data *mydata = hidpp->private_data;
+
+	if ((hidpp->quirks & HIDPP_QUIRK_MULTI_INPUT) && origin_is_hid_core)
+		/* this is the generic hid-input call */
+		return;
+
+	mydata->input = input_dev;
+
+	__set_bit(EV_KEY, mydata->input->evbit);
+	__set_bit(BTN_MIDDLE, mydata->input->keybit);
+	__set_bit(BTN_RIGHT, mydata->input->keybit);
+	__set_bit(BTN_LEFT, mydata->input->keybit);
+	__set_bit(BTN_BACK, mydata->input->keybit);
+	__set_bit(BTN_FORWARD, mydata->input->keybit);
+
+	__set_bit(EV_REL, mydata->input->evbit);
+	__set_bit(REL_X, mydata->input->relbit);
+	__set_bit(REL_Y, mydata->input->relbit);
+	__set_bit(REL_WHEEL, mydata->input->relbit);
+	__set_bit(REL_HWHEEL, mydata->input->relbit);
+
+}
+
+/* -------------------------------------------------------------------------- */
 /* Generic HID++ devices                                                      */
 /* -------------------------------------------------------------------------- */
 
@@ -953,6 +1171,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;
 }
@@ -962,6 +1183,8 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
 {
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		wtp_populate_input(hidpp, input, origin_is_hid_core);
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
+		m560_populate_input(hidpp, input, origin_is_hid_core);
 }
 
 static void hidpp_input_configured(struct hid_device *hdev,
@@ -1049,6 +1272,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;
 }
@@ -1126,6 +1351,8 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 		ret = wtp_connect(hdev, connected);
 		if (ret)
 			return;
+	} else if ((hidpp->quirks & HIDPP_QUIRK_CLASS_M560) && connected) {
+		m560_send_config_command(hdev);
 	}
 
 	if (!connected || hidpp->delayed_input)
@@ -1201,7 +1428,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);
@@ -1268,7 +1500,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;
 }
@@ -1301,6 +1533,11 @@ 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_DELAYED_INIT  |
+			 HIDPP_QUIRK_CLASS_M560 },
 
 	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
-- 
2.1.4