All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] add support for high res wheel found on some Logitech devices
@ 2017-04-07 11:31 Mauro Carvalho Chehab
  2017-04-07 11:31 ` [PATCH v3 1/4] input: add an EV_REL event for high-res vertical wheel Mauro Carvalho Chehab
  0 siblings, 1 reply; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2017-04-07 11:31 UTC (permalink / raw)
  To: linux-input, Dmitry Torokhov; +Cc: Mauro Carvalho Chehab


Some Logitech mouses (MX Anyware 2, MX Master) have a wheel with two
resolutions. It initializes in low resolution mode, but can be changed to high
resolution. It also supports free wheel mode, where there's no tactile
feedback at the wheel, nor it offers resistence to movements. Such mode is
enabled/disabled by clicking at the wheel (on this wheel, the middle button
is a separate button).

Add support for it.

v3: addressed Peter's comments for EV_SW SW_RATCHET documentation

Mauro Carvalho Chehab (4):
  input: add an EV_REL event for high-res vertical wheel
  input: add a EV_SW event for ratchet switch
  hid-logitech-hidpp: add support for high res wheel
  hid-logitech-hidpp: add support for ratchet switch

 Documentation/input/event-codes.rst    |  28 ++++-
 drivers/hid/hid-logitech-hidpp.c       | 218 +++++++++++++++++++++++++++++++++
 include/linux/mod_devicetable.h        |   2 +-
 include/uapi/linux/input-event-codes.h |   5 +-
 4 files changed, 248 insertions(+), 5 deletions(-)

-- 
2.9.3



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/4] input: add an EV_REL event for high-res vertical wheel
  2017-04-07 11:31 [PATCH v3 0/4] add support for high res wheel found on some Logitech devices Mauro Carvalho Chehab
@ 2017-04-07 11:31 ` Mauro Carvalho Chehab
  2017-04-07 11:31   ` [PATCH v3 2/4] input: add a EV_SW event for ratchet switch Mauro Carvalho Chehab
  2017-04-07 12:17   ` [v3,1/4] input: add an EV_REL event for high-res vertical wheel Benjamin Tissoires
  0 siblings, 2 replies; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2017-04-07 11:31 UTC (permalink / raw)
  To: linux-input, Dmitry Torokhov
  Cc: Mauro Carvalho Chehab, Jonathan Corbet, Peter Hutterer,
	Hans Verkuil, Ping Cheng, Kamil Debski, Douglas Anderson,
	linux-doc

As some devices can produce either low-res or high-res
vertical wheel EV_REL events, add a new event to allow
userspace to distinguish between them.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 Documentation/input/event-codes.rst    | 16 +++++++++++++---
 include/uapi/linux/input-event-codes.h |  1 +
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
index 92db50954169..0c8591d39bc6 100644
--- a/Documentation/input/event-codes.rst
+++ b/Documentation/input/event-codes.rst
@@ -185,10 +185,20 @@ instead of EV_REL codes.
 
 A few EV_REL codes have special meanings:
 
-* REL_WHEEL, REL_HWHEEL:
+* REL_WHEEL:
 
-  - These codes are used for vertical and horizontal scroll wheels,
-    respectively.
+  - These codes are used for vertical scroll wheels.
+
+  - REL_WHEEL is for normal wheel operational mode, e. g. low-resolution
+    (line-based) scroll.
+
+  - REL_HIRES_WHEEL should be used when the wheel has two resolutions and it
+    is in high-resolution mode, e. g. the same angular movement that would
+    produce a single REL_WHEEL will produce multiple REL_HIRES_WHEEL events.
+
+* REL_HWHEEL:
+
+  - This code is used for horizontal scroll wheels.
 
 EV_ABS
 ------
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 3af60ee69053..23b2d377af59 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -703,6 +703,7 @@
 #define REL_DIAL		0x07
 #define REL_WHEEL		0x08
 #define REL_MISC		0x09
+#define REL_HIRES_WHEEL		0x0a
 #define REL_MAX			0x0f
 #define REL_CNT			(REL_MAX+1)
 
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/4] input: add a EV_SW event for ratchet switch
  2017-04-07 11:31 ` [PATCH v3 1/4] input: add an EV_REL event for high-res vertical wheel Mauro Carvalho Chehab
@ 2017-04-07 11:31   ` Mauro Carvalho Chehab
  2017-04-07 11:31     ` [PATCH v3 3/4] hid-logitech-hidpp: add support for high res wheel Mauro Carvalho Chehab
  2017-04-07 12:17   ` [v3,1/4] input: add an EV_REL event for high-res vertical wheel Benjamin Tissoires
  1 sibling, 1 reply; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2017-04-07 11:31 UTC (permalink / raw)
  To: linux-input, Dmitry Torokhov
  Cc: Mauro Carvalho Chehab, Jonathan Corbet, Peter Hutterer,
	Florian Fainelli, Ingo Tuchscherer, Stuart Yoder,
	Greg Kroah-Hartman, Hans Verkuil, Kamil Debski, Ping Cheng,
	Douglas Anderson, linux-doc

Some mice have a switch on their wheel, allowing to switch
between ratchet and free wheel mode. Add support for it.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 Documentation/input/event-codes.rst    | 12 ++++++++++++
 include/linux/mod_devicetable.h        |  2 +-
 include/uapi/linux/input-event-codes.h |  4 +++-
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
index 0c8591d39bc6..4cb7eab73e78 100644
--- a/Documentation/input/event-codes.rst
+++ b/Documentation/input/event-codes.rst
@@ -239,6 +239,18 @@ Upon resume, if the switch state is the same as before suspend, then the input
 subsystem will filter out the duplicate switch state reports. The driver does
 not need to keep the state of the switch at any time.
 
+A few EV_SW codes have special meanings:
+
+* SW_RATCHET:
+
+  - Some mice have a special switch for their wheel that allows to change
+    between free wheel mode and ratchet mode. When the switch is ratchet
+    mode (ON state), the wheel will offer some resistance for movements. It
+    may also provide a tactile feedback when scrolled.
+
+    Note that some mice have a ratchet switch that does not generate a
+    software event.
+
 EV_MSC
 ------
 
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 8850fcaf50db..038cddf1436a 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -292,7 +292,7 @@ struct pcmcia_device_id {
 #define INPUT_DEVICE_ID_LED_MAX		0x0f
 #define INPUT_DEVICE_ID_SND_MAX		0x07
 #define INPUT_DEVICE_ID_FF_MAX		0x7f
-#define INPUT_DEVICE_ID_SW_MAX		0x0f
+#define INPUT_DEVICE_ID_SW_MAX		0x1f
 
 #define INPUT_DEVICE_ID_MATCH_BUS	1
 #define INPUT_DEVICE_ID_MATCH_VENDOR	2
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 23b2d377af59..a3eafd0527f1 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -782,7 +782,9 @@
 #define SW_LINEIN_INSERT	0x0d  /* set = inserted */
 #define SW_MUTE_DEVICE		0x0e  /* set = device disabled */
 #define SW_PEN_INSERTED		0x0f  /* set = pen inserted */
-#define SW_MAX			0x0f
+#define SW_RATCHET		0x10  /* set = ratchet mode,
+					 unset: free wheel */
+#define SW_MAX			0x1f
 #define SW_CNT			(SW_MAX+1)
 
 /*
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 3/4] hid-logitech-hidpp: add support for high res wheel
  2017-04-07 11:31   ` [PATCH v3 2/4] input: add a EV_SW event for ratchet switch Mauro Carvalho Chehab
@ 2017-04-07 11:31     ` Mauro Carvalho Chehab
  2017-04-07 11:31       ` [PATCH v3 4/4] hid-logitech-hidpp: add support for ratchet switch Mauro Carvalho Chehab
  2017-04-07 15:31       ` [PATCH v3 3/4] hid-logitech-hidpp: add support for high res wheel Benjamin Tissoires
  0 siblings, 2 replies; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2017-04-07 11:31 UTC (permalink / raw)
  To: linux-input, Dmitry Torokhov
  Cc: Mauro Carvalho Chehab, Jiri Kosina, Benjamin Tissoires

Some Logitech mouses (MX Anyware 2 and MX Master) have support
for a high-resolution wheel.

This wheel can work in backward-compatible mode, generating
wheel events via HID normal events, or it can use new
HID++ events that report not only the wheel movement, but also
the resolution.

Add support for it.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 drivers/hid/hid-logitech-hidpp.c | 199 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 199 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 2e2515a4c070..c208a5107511 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -62,6 +62,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
 #define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS	BIT(22)
 #define HIDPP_QUIRK_NO_HIDINPUT			BIT(23)
 #define HIDPP_QUIRK_FORCE_OUTPUT_REPORTS	BIT(24)
+#define HIDPP_QUIRK_HIRES_SCROLL		BIT(25)
 
 #define HIDPP_QUIRK_DELAYED_INIT		(HIDPP_QUIRK_NO_HIDINPUT | \
 						 HIDPP_QUIRK_CONNECT_EVENTS)
@@ -1361,6 +1362,67 @@ static int hidpp_ff_deinit(struct hid_device *hid)
 	return 0;
 }
 
+/* -------------------------------------------------------------------------- */
+/* 0x2121: High Resolution Wheel                                              */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_HIGH_RES_WHEEL		0x2121
+
+#define CMD_MOUSE_SET_WHEEL_MODE	0x20
+#define CMD_MOUSE_GET_WHEEL_RATCHET	0x30
+
+struct high_res_wheel_data {
+	u8 feature_index;
+	struct input_dev *input;
+	bool ratchet;
+};
+
+/**
+ * hidpp_mouse_set_wheel_mode - Sets high resolution wheel mode
+ *
+ * @invert:	if true, inverts wheel movement
+ * @high_res:	if true, wheel is in high-resolution mode. Otherwise, low res
+ * @hidpp:	if true, report wheel events via HID++ notification. If false,
+ *		use standard HID events
+ */
+static int hidpp_mouse_set_wheel_mode(struct hidpp_device *hidpp,
+				      bool invert,
+				      bool high_res,
+				      bool hidpp_mode)
+{
+	struct high_res_wheel_data *hrd = hidpp->private_data;
+	u8 feature_type;
+	struct hidpp_report response;
+	int ret;
+	u8 params[16] = { 0 };
+
+	if (!hrd->feature_index) {
+		ret = hidpp_root_get_feature(hidpp,
+					    HIDPP_HIGH_RES_WHEEL,
+					    &hrd->feature_index,
+					    &feature_type);
+		if (ret)
+			/* means that the device is not powered up */
+			return ret;
+	}
+
+	params[0] = invert     ? 0x4 : 0  |
+		    high_res   ? 0x2 : 0  |
+		    hidpp_mode ? 0x1 : 0;
+
+	ret = hidpp_send_fap_command_sync(hidpp, hrd->feature_index,
+					  CMD_MOUSE_SET_WHEEL_MODE,
+					  params, 16, &response);
+	if (ret > 0) {
+		hid_err(hidpp->hid_dev, "%s: received protocol error 0x%02x\n",
+			__func__, ret);
+		return -EPROTO;
+	}
+	if (ret)
+		return ret;
+
+	return 0;
+}
 
 /* ************************************************************************** */
 /*                                                                            */
@@ -1816,6 +1878,119 @@ static int m560_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 }
 
 /* ------------------------------------------------------------------------- */
+/* Logitech mouse devices with high resolution wheel                         */
+/* ------------------------------------------------------------------------- */
+
+static int high_res_raw_event(struct hid_device *hdev, u8 *data, int size)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+	struct high_res_wheel_data *hrd = hidpp->private_data;
+
+	/* Don't handle special raw events before setting feature_index */
+	if (!hrd || !hrd->feature_index)
+		return 0;
+
+	if (data[0] != REPORT_ID_HIDPP_LONG ||
+	    data[2] != hrd->feature_index)
+		return 1;
+
+	if (size < 8) {
+		hid_err(hdev, "error in report: size = %d: %*ph\n", size,
+			size, data);
+		return 0;
+	}
+
+	/*
+	 * high res wheel mouse events
+	 *
+	 * Wheel movement events are like:
+	 *
+	 * 11 03 0b 00 01 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
+	 *
+	 * data[0] = 0x11
+	 * data[1] = device-id
+	 * data[2] = feature index (0b)
+	 * data[3] = event type: 0x00 - wheel movement
+	 * data[4] = bitmask:
+	 *		bits 0-3: number of sampling periods combined
+	 *		bit 4:
+	 *			0 = low resolution
+	 *			1 = high resolution
+	 * data[5] - deltaV MSB
+	 * data[6] = deltaV LSB
+	 * Remaining payload is reserved
+	 *
+	 * Ratchet events are like:
+	 * 11 03 0b 10 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+	 *
+	 * data[0] = 0x11
+	 * data[1] = device-id
+	 * data[2] = feature index
+	 * data[3] = event type: 0x10 - ratchet state
+	 * data[4] = bit 0:
+	 *		1 = ratchet
+	 *		0 = free wheel
+	 * Remaining payload is reserved
+	 */
+
+	if (data[3] == 0) {
+		s16 delta = data[6] | data[5] << 8;
+		bool res = data[4] & 0x10;
+
+		/*
+		 * Report high-resolution events as REL_HWHEEL and
+		 * low-resolution events as REL_WHEEL.
+		 */
+		if (res)
+			input_report_rel(hrd->input, REL_HIRES_WHEEL, delta);
+		else
+			input_report_rel(hrd->input, REL_WHEEL, delta);
+	}
+
+	/* FIXME: also report ratchet events to userspace */
+
+	return 1;
+}
+
+static void high_res_populate_input(struct hidpp_device *hidpp,
+		struct input_dev *input_dev, bool origin_is_hid_core)
+{
+	struct high_res_wheel_data *hrd = hidpp->private_data;
+
+	hrd->input = input_dev;
+
+	__set_bit(REL_WHEEL, hrd->input->relbit);
+	__set_bit(REL_HIRES_WHEEL, hrd->input->relbit);
+}
+
+
+static int high_res_allocate(struct hid_device *hdev)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+	struct high_res_wheel_data *hrd;
+
+	hrd = devm_kzalloc(&hdev->dev, sizeof(struct high_res_wheel_data),
+			GFP_KERNEL);
+	if (!hrd)
+		return -ENOMEM;
+
+	hidpp->private_data = hrd;
+
+	return 0;
+};
+
+static int high_res_connect(struct hid_device *hdev, bool connected)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+
+	if (!connected)
+		return 0;
+
+	/* Enable HID++ wheel event output mode */
+	return hidpp_mouse_set_wheel_mode(hidpp, false, false, true);
+}
+
+/* ------------------------------------------------------------------------- */
 /* Logitech K400 devices                                                     */
 /* ------------------------------------------------------------------------- */
 
@@ -1955,6 +2130,9 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
 		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);
+	else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL)
+		high_res_populate_input(hidpp, input, origin_is_hid_core);
+
 }
 
 static int hidpp_input_configured(struct hid_device *hdev,
@@ -2054,6 +2232,8 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
 		return wtp_raw_event(hdev, data, size);
 	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
 		return m560_raw_event(hdev, data, size);
+	else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL)
+		return high_res_raw_event(hdev, data, size);
 
 	return 0;
 }
@@ -2141,6 +2321,10 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 		ret = k400_connect(hdev, connected);
 		if (ret)
 			return;
+	} else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL) {
+		ret = high_res_connect(hdev, connected);
+		if (ret)
+			return;
 	}
 
 	if (!connected || hidpp->delayed_input)
@@ -2215,6 +2399,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		hidpp->quirks &= ~HIDPP_QUIRK_CLASS_WTP;
 		hidpp->quirks &= ~HIDPP_QUIRK_CONNECT_EVENTS;
 		hidpp->quirks &= ~HIDPP_QUIRK_NO_HIDINPUT;
+		hidpp->quirks &= ~HIDPP_QUIRK_HIRES_SCROLL;
 	}
 
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
@@ -2229,6 +2414,10 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		ret = k400_allocate(hdev);
 		if (ret)
 			goto allocate_fail;
+	} else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL) {
+		ret = high_res_allocate(hdev);
+		if (ret)
+			goto allocate_fail;
 	}
 
 	INIT_WORK(&hidpp->work, delayed_work_cb);
@@ -2354,6 +2543,16 @@ static const struct hid_device_id hidpp_devices[] = {
 	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, 0x402d),
 	  .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
+	{ /* Logitech MX Master with high resolution scroll */
+	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
+		USB_VENDOR_ID_LOGITECH, 0x4041),
+	  .driver_data = HIDPP_QUIRK_CONNECT_EVENTS |
+			 HIDPP_QUIRK_HIRES_SCROLL },
+	{ /* Logitech MX Anywhere 2 with high resolution scroll */
+	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
+		USB_VENDOR_ID_LOGITECH, 0x404a),
+	  .driver_data = HIDPP_QUIRK_CONNECT_EVENTS |
+			 HIDPP_QUIRK_HIRES_SCROLL },
 	{ /* Keyboard logitech K400 */
 	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, 0x4024),
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 4/4] hid-logitech-hidpp: add support for ratchet switch
  2017-04-07 11:31     ` [PATCH v3 3/4] hid-logitech-hidpp: add support for high res wheel Mauro Carvalho Chehab
@ 2017-04-07 11:31       ` Mauro Carvalho Chehab
  2017-04-07 15:33         ` Benjamin Tissoires
  2017-04-07 15:31       ` [PATCH v3 3/4] hid-logitech-hidpp: add support for high res wheel Benjamin Tissoires
  1 sibling, 1 reply; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2017-04-07 11:31 UTC (permalink / raw)
  To: linux-input, Dmitry Torokhov
  Cc: Mauro Carvalho Chehab, Jiri Kosina, Benjamin Tissoires

Logitech Anywhere MX2 and MX master produce events for
the wheel ratchet/free wheel button. Add support for it.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 drivers/hid/hid-logitech-hidpp.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index c208a5107511..6177087ffaca 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1406,6 +1406,19 @@ static int hidpp_mouse_set_wheel_mode(struct hidpp_device *hidpp,
 			return ret;
 	}
 
+	ret = hidpp_send_fap_command_sync(hidpp, hrd->feature_index,
+					  CMD_MOUSE_GET_WHEEL_RATCHET,
+					  params, 16, &response);
+	if (ret > 0) {
+		hid_err(hidpp->hid_dev, "%s: received protocol error 0x%02x\n",
+			__func__, ret);
+		return -EPROTO;
+	}
+	if (ret)
+		return ret;
+
+	hrd->ratchet = response.fap.params[0] & 0x01;
+
 	params[0] = invert     ? 0x4 : 0  |
 		    high_res   ? 0x2 : 0  |
 		    hidpp_mode ? 0x1 : 0;
@@ -1945,10 +1958,11 @@ static int high_res_raw_event(struct hid_device *hdev, u8 *data, int size)
 			input_report_rel(hrd->input, REL_HIRES_WHEEL, delta);
 		else
 			input_report_rel(hrd->input, REL_WHEEL, delta);
+	} else if (data[3] == 0x10) {
+		hrd->ratchet = data[4] & 0x01;
+		input_report_switch(hrd->input, SW_RATCHET, hrd->ratchet);
 	}
 
-	/* FIXME: also report ratchet events to userspace */
-
 	return 1;
 }
 
@@ -1961,6 +1975,11 @@ static void high_res_populate_input(struct hidpp_device *hidpp,
 
 	__set_bit(REL_WHEEL, hrd->input->relbit);
 	__set_bit(REL_HIRES_WHEEL, hrd->input->relbit);
+	__set_bit(EV_SW, hrd->input->evbit);
+	__set_bit(SW_RATCHET, hrd->input->swbit);
+
+	/* Report current state of the ratchet switch */
+	input_report_switch(hrd->input, SW_RATCHET, hrd->ratchet);
 }
 
 
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [v3,1/4] input: add an EV_REL event for high-res vertical wheel
  2017-04-07 11:31 ` [PATCH v3 1/4] input: add an EV_REL event for high-res vertical wheel Mauro Carvalho Chehab
  2017-04-07 11:31   ` [PATCH v3 2/4] input: add a EV_SW event for ratchet switch Mauro Carvalho Chehab
@ 2017-04-07 12:17   ` Benjamin Tissoires
  2017-04-07 13:32     ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 11+ messages in thread
From: Benjamin Tissoires @ 2017-04-07 12:17 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-input, Dmitry Torokhov, Jonathan Corbet, Peter Hutterer,
	Hans Verkuil, Ping Cheng, Kamil Debski, Douglas Anderson,
	linux-doc

Hi Mauro,

On Apr 07 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> As some devices can produce either low-res or high-res
> vertical wheel EV_REL events, add a new event to allow
> userspace to distinguish between them.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
>  Documentation/input/event-codes.rst    | 16 +++++++++++++---
>  include/uapi/linux/input-event-codes.h |  1 +
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
> index 92db50954169..0c8591d39bc6 100644
> --- a/Documentation/input/event-codes.rst
> +++ b/Documentation/input/event-codes.rst
> @@ -185,10 +185,20 @@ instead of EV_REL codes.
>  
>  A few EV_REL codes have special meanings:
>  
> -* REL_WHEEL, REL_HWHEEL:
> +* REL_WHEEL:
>  
> -  - These codes are used for vertical and horizontal scroll wheels,
> -    respectively.
> +  - These codes are used for vertical scroll wheels.
> +
> +  - REL_WHEEL is for normal wheel operational mode, e. g. low-resolution
> +    (line-based) scroll.
> +
> +  - REL_HIRES_WHEEL should be used when the wheel has two resolutions and it
> +    is in high-resolution mode, e. g. the same angular movement that would
> +    produce a single REL_WHEEL will produce multiple REL_HIRES_WHEEL events.
> +
> +* REL_HWHEEL:
> +
> +  - This code is used for horizontal scroll wheels.
>  
>  EV_ABS
>  ------
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 3af60ee69053..23b2d377af59 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -703,6 +703,7 @@
>  #define REL_DIAL		0x07
>  #define REL_WHEEL		0x08
>  #define REL_MISC		0x09
> +#define REL_HIRES_WHEEL		0x0a

There is one issue here. The HID input layer has a tendency to map
usages to REL_MISC +1, +2, +3, etc... When it doesn't know how to map an
usage, the core layer maps it to the next one.

I am not aware of devices exposing such REL axes, but I wouldn't be
surprised if the issue will not raise at some point.

We already had an issue with ABS events where ABS_MISC are conflicting
with ABS_MT_SLOTS. Luckily, we have a little empty space between
ABS_MISC and ABS_MT_SLOT, which allows userspace to detect such issues.

Could you give the define a bigger number (and maybe reserve a couple of
REL_MISC-n) to make sure that we do not end up with joysticks exporting
REL_HIRES_WHEEL?

I'll try the rest of the series on the MX Master today.

Cheers,
Benjamin

>  #define REL_MAX			0x0f
>  #define REL_CNT			(REL_MAX+1)
>  

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [v3,1/4] input: add an EV_REL event for high-res vertical wheel
  2017-04-07 12:17   ` [v3,1/4] input: add an EV_REL event for high-res vertical wheel Benjamin Tissoires
@ 2017-04-07 13:32     ` Mauro Carvalho Chehab
  2017-04-07 15:10       ` Benjamin Tissoires
  0 siblings, 1 reply; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2017-04-07 13:32 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: linux-input, Dmitry Torokhov, Jonathan Corbet, Peter Hutterer,
	Hans Verkuil, Ping Cheng, Kamil Debski, Douglas Anderson,
	linux-doc

Em Fri, 7 Apr 2017 14:17:51 +0200
Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:

> Hi Mauro,
> 
> On Apr 07 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> > As some devices can produce either low-res or high-res
> > vertical wheel EV_REL events, add a new event to allow
> > userspace to distinguish between them.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > ---
> >  Documentation/input/event-codes.rst    | 16 +++++++++++++---
> >  include/uapi/linux/input-event-codes.h |  1 +
> >  2 files changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
> > index 92db50954169..0c8591d39bc6 100644
> > --- a/Documentation/input/event-codes.rst
> > +++ b/Documentation/input/event-codes.rst
> > @@ -185,10 +185,20 @@ instead of EV_REL codes.
> >  
> >  A few EV_REL codes have special meanings:
> >  
> > -* REL_WHEEL, REL_HWHEEL:
> > +* REL_WHEEL:
> >  
> > -  - These codes are used for vertical and horizontal scroll wheels,
> > -    respectively.
> > +  - These codes are used for vertical scroll wheels.
> > +
> > +  - REL_WHEEL is for normal wheel operational mode, e. g. low-resolution
> > +    (line-based) scroll.
> > +
> > +  - REL_HIRES_WHEEL should be used when the wheel has two resolutions and it
> > +    is in high-resolution mode, e. g. the same angular movement that would
> > +    produce a single REL_WHEEL will produce multiple REL_HIRES_WHEEL events.
> > +
> > +* REL_HWHEEL:
> > +
> > +  - This code is used for horizontal scroll wheels.
> >  
> >  EV_ABS
> >  ------
> > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > index 3af60ee69053..23b2d377af59 100644
> > --- a/include/uapi/linux/input-event-codes.h
> > +++ b/include/uapi/linux/input-event-codes.h
> > @@ -703,6 +703,7 @@
> >  #define REL_DIAL		0x07
> >  #define REL_WHEEL		0x08
> >  #define REL_MISC		0x09
> > +#define REL_HIRES_WHEEL		0x0a  
> 
> There is one issue here. The HID input layer has a tendency to map
> usages to REL_MISC +1, +2, +3, etc... When it doesn't know how to map an
> usage, the core layer maps it to the next one.
> 
> I am not aware of devices exposing such REL axes, but I wouldn't be
> surprised if the issue will not raise at some point.
> 
> We already had an issue with ABS events where ABS_MISC are conflicting
> with ABS_MT_SLOTS. Luckily, we have a little empty space between
> ABS_MISC and ABS_MT_SLOT, which allows userspace to detect such issues.
> 
> Could you give the define a bigger number (and maybe reserve a couple of
> REL_MISC-n) to make sure that we do not end up with joysticks exporting
> REL_HIRES_WHEEL?

Sure. How many REL_MISC-n should be reserved?

> I'll try the rest of the series on the MX Master today.

It would be great if you could test it!

Regards,
Mauro

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [v3,1/4] input: add an EV_REL event for high-res vertical wheel
  2017-04-07 13:32     ` Mauro Carvalho Chehab
@ 2017-04-07 15:10       ` Benjamin Tissoires
  2017-04-11 10:17         ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 11+ messages in thread
From: Benjamin Tissoires @ 2017-04-07 15:10 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-input, Dmitry Torokhov, Jonathan Corbet, Peter Hutterer,
	Hans Verkuil, Ping Cheng, Kamil Debski, Douglas Anderson,
	linux-doc

On Apr 07 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> Em Fri, 7 Apr 2017 14:17:51 +0200
> Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:
> 
> > Hi Mauro,
> > 
> > On Apr 07 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> > > As some devices can produce either low-res or high-res
> > > vertical wheel EV_REL events, add a new event to allow
> > > userspace to distinguish between them.
> > > 
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > > ---
> > >  Documentation/input/event-codes.rst    | 16 +++++++++++++---
> > >  include/uapi/linux/input-event-codes.h |  1 +
> > >  2 files changed, 14 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
> > > index 92db50954169..0c8591d39bc6 100644
> > > --- a/Documentation/input/event-codes.rst
> > > +++ b/Documentation/input/event-codes.rst
> > > @@ -185,10 +185,20 @@ instead of EV_REL codes.
> > >  
> > >  A few EV_REL codes have special meanings:
> > >  
> > > -* REL_WHEEL, REL_HWHEEL:
> > > +* REL_WHEEL:
> > >  
> > > -  - These codes are used for vertical and horizontal scroll wheels,
> > > -    respectively.
> > > +  - These codes are used for vertical scroll wheels.
> > > +
> > > +  - REL_WHEEL is for normal wheel operational mode, e. g. low-resolution
> > > +    (line-based) scroll.
> > > +
> > > +  - REL_HIRES_WHEEL should be used when the wheel has two resolutions and it
> > > +    is in high-resolution mode, e. g. the same angular movement that would
> > > +    produce a single REL_WHEEL will produce multiple REL_HIRES_WHEEL events.
> > > +
> > > +* REL_HWHEEL:
> > > +
> > > +  - This code is used for horizontal scroll wheels.
> > >  
> > >  EV_ABS
> > >  ------
> > > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > > index 3af60ee69053..23b2d377af59 100644
> > > --- a/include/uapi/linux/input-event-codes.h
> > > +++ b/include/uapi/linux/input-event-codes.h
> > > @@ -703,6 +703,7 @@
> > >  #define REL_DIAL		0x07
> > >  #define REL_WHEEL		0x08
> > >  #define REL_MISC		0x09
> > > +#define REL_HIRES_WHEEL		0x0a  
> > 
> > There is one issue here. The HID input layer has a tendency to map
> > usages to REL_MISC +1, +2, +3, etc... When it doesn't know how to map an
> > usage, the core layer maps it to the next one.
> > 
> > I am not aware of devices exposing such REL axes, but I wouldn't be
> > surprised if the issue will not raise at some point.
> > 
> > We already had an issue with ABS events where ABS_MISC are conflicting
> > with ABS_MT_SLOTS. Luckily, we have a little empty space between
> > ABS_MISC and ABS_MT_SLOT, which allows userspace to detect such issues.
> > 
> > Could you give the define a bigger number (and maybe reserve a couple of
> > REL_MISC-n) to make sure that we do not end up with joysticks exporting
> > REL_HIRES_WHEEL?
> 
> Sure. How many REL_MISC-n should be reserved?

Not sure. As I mentioned, I haven' t seen such device myself, but maybe
we should reserve the rest of the 0x0n range for those (0x0a-0x0f). This
would force to bump REL_MAX, but I don't think this is an issue besides
recompiling libevdev and others.

> 
> > I'll try the rest of the series on the MX Master today.
> 
> It would be great if you could test it!

I found some issues, I'll report in the other patches.

Cheers,
Benjamin

> 
> Regards,
> Mauro

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 3/4] hid-logitech-hidpp: add support for high res wheel
  2017-04-07 11:31     ` [PATCH v3 3/4] hid-logitech-hidpp: add support for high res wheel Mauro Carvalho Chehab
  2017-04-07 11:31       ` [PATCH v3 4/4] hid-logitech-hidpp: add support for ratchet switch Mauro Carvalho Chehab
@ 2017-04-07 15:31       ` Benjamin Tissoires
  1 sibling, 0 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2017-04-07 15:31 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-input, Dmitry Torokhov, Jiri Kosina

Hi Mauro,

On Apr 07 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> Some Logitech mouses (MX Anyware 2 and MX Master) have support
> for a high-resolution wheel.
> 
> This wheel can work in backward-compatible mode, generating
> wheel events via HID normal events, or it can use new
> HID++ events that report not only the wheel movement, but also
> the resolution.
> 
> Add support for it.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---

Please rebase the patch on top of the HID tree 'for-next' branch:
https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git/

>  drivers/hid/hid-logitech-hidpp.c | 199 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 199 insertions(+)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 2e2515a4c070..c208a5107511 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -62,6 +62,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
>  #define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS	BIT(22)
>  #define HIDPP_QUIRK_NO_HIDINPUT			BIT(23)
>  #define HIDPP_QUIRK_FORCE_OUTPUT_REPORTS	BIT(24)
> +#define HIDPP_QUIRK_HIRES_SCROLL		BIT(25)

There has been some addition in the HID tree there, so you'll need to
change the value.

>  
>  #define HIDPP_QUIRK_DELAYED_INIT		(HIDPP_QUIRK_NO_HIDINPUT | \
>  						 HIDPP_QUIRK_CONNECT_EVENTS)
> @@ -1361,6 +1362,67 @@ static int hidpp_ff_deinit(struct hid_device *hid)
>  	return 0;
>  }
>  
> +/* -------------------------------------------------------------------------- */
> +/* 0x2121: High Resolution Wheel                                              */
> +/* -------------------------------------------------------------------------- */
> +
> +#define HIDPP_HIGH_RES_WHEEL		0x2121
> +
> +#define CMD_MOUSE_SET_WHEEL_MODE	0x20
> +#define CMD_MOUSE_GET_WHEEL_RATCHET	0x30
> +
> +struct high_res_wheel_data {
> +	u8 feature_index;
> +	struct input_dev *input;
> +	bool ratchet;
> +};
> +
> +/**
> + * hidpp_mouse_set_wheel_mode - Sets high resolution wheel mode
> + *
> + * @invert:	if true, inverts wheel movement
> + * @high_res:	if true, wheel is in high-resolution mode. Otherwise, low res
> + * @hidpp:	if true, report wheel events via HID++ notification. If false,
> + *		use standard HID events
> + */
> +static int hidpp_mouse_set_wheel_mode(struct hidpp_device *hidpp,
> +				      bool invert,
> +				      bool high_res,
> +				      bool hidpp_mode)
> +{
> +	struct high_res_wheel_data *hrd = hidpp->private_data;
> +	u8 feature_type;
> +	struct hidpp_report response;
> +	int ret;
> +	u8 params[16] = { 0 };
> +
> +	if (!hrd->feature_index) {
> +		ret = hidpp_root_get_feature(hidpp,
> +					    HIDPP_HIGH_RES_WHEEL,
> +					    &hrd->feature_index,
> +					    &feature_type);
> +		if (ret)
> +			/* means that the device is not powered up */
> +			return ret;
> +	}
> +
> +	params[0] = invert     ? 0x4 : 0  |
> +		    high_res   ? 0x2 : 0  |
> +		    hidpp_mode ? 0x1 : 0;

It took me a while, but I finally understood why the behavior was not
working properly. I ended up adding braces (and use of BIT macro):
	params[0] = (invert     ? BIT(2) : 0)  |
		    (high_res   ? BIT(1) : 0)  |
		    (hidpp_mode ? BIT(0) : 0);

> +
> +	ret = hidpp_send_fap_command_sync(hidpp, hrd->feature_index,
> +					  CMD_MOUSE_SET_WHEEL_MODE,
> +					  params, 16, &response);

No need to send 16 parameters when only one is used. Replacing 16 by 1
just works.

> +	if (ret > 0) {
> +		hid_err(hidpp->hid_dev, "%s: received protocol error 0x%02x\n",
> +			__func__, ret);
> +		return -EPROTO;
> +	}
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
>  
>  /* ************************************************************************** */
>  /*                                                                            */
> @@ -1816,6 +1878,119 @@ static int m560_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  }
>  
>  /* ------------------------------------------------------------------------- */
> +/* Logitech mouse devices with high resolution wheel                         */
> +/* ------------------------------------------------------------------------- */
> +
> +static int high_res_raw_event(struct hid_device *hdev, u8 *data, int size)
> +{
> +	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> +	struct high_res_wheel_data *hrd = hidpp->private_data;
> +
> +	/* Don't handle special raw events before setting feature_index */
> +	if (!hrd || !hrd->feature_index)
> +		return 0;
> +
> +	if (data[0] != REPORT_ID_HIDPP_LONG ||
> +	    data[2] != hrd->feature_index)
> +		return 1;
> +
> +	if (size < 8) {
> +		hid_err(hdev, "error in report: size = %d: %*ph\n", size,
> +			size, data);
> +		return 0;
> +	}
> +
> +	/*
> +	 * high res wheel mouse events
> +	 *
> +	 * Wheel movement events are like:
> +	 *
> +	 * 11 03 0b 00 01 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
> +	 *
> +	 * data[0] = 0x11
> +	 * data[1] = device-id
> +	 * data[2] = feature index (0b)
> +	 * data[3] = event type: 0x00 - wheel movement
> +	 * data[4] = bitmask:
> +	 *		bits 0-3: number of sampling periods combined
> +	 *		bit 4:
> +	 *			0 = low resolution
> +	 *			1 = high resolution
> +	 * data[5] - deltaV MSB
> +	 * data[6] = deltaV LSB
> +	 * Remaining payload is reserved
> +	 *
> +	 * Ratchet events are like:
> +	 * 11 03 0b 10 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +	 *
> +	 * data[0] = 0x11
> +	 * data[1] = device-id
> +	 * data[2] = feature index
> +	 * data[3] = event type: 0x10 - ratchet state
> +	 * data[4] = bit 0:
> +	 *		1 = ratchet
> +	 *		0 = free wheel
> +	 * Remaining payload is reserved
> +	 */
> +
> +	if (data[3] == 0) {
> +		s16 delta = data[6] | data[5] << 8;
> +		bool res = data[4] & 0x10;
> +
> +		/*
> +		 * Report high-resolution events as REL_HWHEEL and
> +		 * low-resolution events as REL_WHEEL.
> +		 */
> +		if (res)
> +			input_report_rel(hrd->input, REL_HIRES_WHEEL, delta);
> +		else
> +			input_report_rel(hrd->input, REL_WHEEL, delta);
> +	}
> +
> +	/* FIXME: also report ratchet events to userspace */
> +
> +	return 1;
> +}
> +
> +static void high_res_populate_input(struct hidpp_device *hidpp,
> +		struct input_dev *input_dev, bool origin_is_hid_core)
> +{
> +	struct high_res_wheel_data *hrd = hidpp->private_data;
> +
> +	hrd->input = input_dev;
> +
> +	__set_bit(REL_WHEEL, hrd->input->relbit);
> +	__set_bit(REL_HIRES_WHEEL, hrd->input->relbit);
> +}
> +
> +
> +static int high_res_allocate(struct hid_device *hdev)
> +{
> +	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> +	struct high_res_wheel_data *hrd;
> +
> +	hrd = devm_kzalloc(&hdev->dev, sizeof(struct high_res_wheel_data),
> +			GFP_KERNEL);
> +	if (!hrd)
> +		return -ENOMEM;
> +
> +	hidpp->private_data = hrd;
> +
> +	return 0;
> +};
> +
> +static int high_res_connect(struct hid_device *hdev, bool connected)
> +{
> +	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> +
> +	if (!connected)
> +		return 0;
> +
> +	/* Enable HID++ wheel event output mode */
> +	return hidpp_mouse_set_wheel_mode(hidpp, false, false, true);
> +}
> +
> +/* ------------------------------------------------------------------------- */
>  /* Logitech K400 devices                                                     */
>  /* ------------------------------------------------------------------------- */
>  
> @@ -1955,6 +2130,9 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
>  		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);
> +	else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL)
> +		high_res_populate_input(hidpp, input, origin_is_hid_core);
> +
>  }
>  
>  static int hidpp_input_configured(struct hid_device *hdev,
> @@ -2054,6 +2232,8 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
>  		return wtp_raw_event(hdev, data, size);
>  	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
>  		return m560_raw_event(hdev, data, size);
> +	else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL)
> +		return high_res_raw_event(hdev, data, size);
>  
>  	return 0;
>  }
> @@ -2141,6 +2321,10 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
>  		ret = k400_connect(hdev, connected);
>  		if (ret)
>  			return;
> +	} else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL) {
> +		ret = high_res_connect(hdev, connected);
> +		if (ret)
> +			return;
>  	}
>  
>  	if (!connected || hidpp->delayed_input)
> @@ -2215,6 +2399,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		hidpp->quirks &= ~HIDPP_QUIRK_CLASS_WTP;
>  		hidpp->quirks &= ~HIDPP_QUIRK_CONNECT_EVENTS;
>  		hidpp->quirks &= ~HIDPP_QUIRK_NO_HIDINPUT;
> +		hidpp->quirks &= ~HIDPP_QUIRK_HIRES_SCROLL;
>  	}
>  
>  	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
> @@ -2229,6 +2414,10 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		ret = k400_allocate(hdev);
>  		if (ret)
>  			goto allocate_fail;
> +	} else if (hidpp->quirks & HIDPP_QUIRK_HIRES_SCROLL) {
> +		ret = high_res_allocate(hdev);
> +		if (ret)
> +			goto allocate_fail;
>  	}
>  
>  	INIT_WORK(&hidpp->work, delayed_work_cb);
> @@ -2354,6 +2543,16 @@ static const struct hid_device_id hidpp_devices[] = {
>  	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
>  		USB_VENDOR_ID_LOGITECH, 0x402d),
>  	  .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
> +	{ /* Logitech MX Master with high resolution scroll */
> +	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
> +		USB_VENDOR_ID_LOGITECH, 0x4041),
> +	  .driver_data = HIDPP_QUIRK_CONNECT_EVENTS |

HIDPP_QUIRK_CONNECT_EVENTS is now applied by default, so there is no
need to add it.

> +			 HIDPP_QUIRK_HIRES_SCROLL },
> +	{ /* Logitech MX Anywhere 2 with high resolution scroll */
> +	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
> +		USB_VENDOR_ID_LOGITECH, 0x404a),
> +	  .driver_data = HIDPP_QUIRK_CONNECT_EVENTS |
> +			 HIDPP_QUIRK_HIRES_SCROLL },
>  	{ /* Keyboard logitech K400 */
>  	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
>  		USB_VENDOR_ID_LOGITECH, 0x4024),
> -- 
> 2.9.3
> 

Cheers,
Benjamin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 4/4] hid-logitech-hidpp: add support for ratchet switch
  2017-04-07 11:31       ` [PATCH v3 4/4] hid-logitech-hidpp: add support for ratchet switch Mauro Carvalho Chehab
@ 2017-04-07 15:33         ` Benjamin Tissoires
  0 siblings, 0 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2017-04-07 15:33 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-input, Dmitry Torokhov, Jiri Kosina

On Apr 07 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> Logitech Anywhere MX2 and MX master produce events for
> the wheel ratchet/free wheel button. Add support for it.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
>  drivers/hid/hid-logitech-hidpp.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index c208a5107511..6177087ffaca 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -1406,6 +1406,19 @@ static int hidpp_mouse_set_wheel_mode(struct hidpp_device *hidpp,
>  			return ret;
>  	}
>  
> +	ret = hidpp_send_fap_command_sync(hidpp, hrd->feature_index,
> +					  CMD_MOUSE_GET_WHEEL_RATCHET,
> +					  params, 16, &response);

Like in the other patch, there is no need to have '16' here when there
is no parameters.

Cheers,
Benjamin

> +	if (ret > 0) {
> +		hid_err(hidpp->hid_dev, "%s: received protocol error 0x%02x\n",
> +			__func__, ret);
> +		return -EPROTO;
> +	}
> +	if (ret)
> +		return ret;
> +
> +	hrd->ratchet = response.fap.params[0] & 0x01;
> +
>  	params[0] = invert     ? 0x4 : 0  |
>  		    high_res   ? 0x2 : 0  |
>  		    hidpp_mode ? 0x1 : 0;
> @@ -1945,10 +1958,11 @@ static int high_res_raw_event(struct hid_device *hdev, u8 *data, int size)
>  			input_report_rel(hrd->input, REL_HIRES_WHEEL, delta);
>  		else
>  			input_report_rel(hrd->input, REL_WHEEL, delta);
> +	} else if (data[3] == 0x10) {
> +		hrd->ratchet = data[4] & 0x01;
> +		input_report_switch(hrd->input, SW_RATCHET, hrd->ratchet);
>  	}
>  
> -	/* FIXME: also report ratchet events to userspace */
> -
>  	return 1;
>  }
>  
> @@ -1961,6 +1975,11 @@ static void high_res_populate_input(struct hidpp_device *hidpp,
>  
>  	__set_bit(REL_WHEEL, hrd->input->relbit);
>  	__set_bit(REL_HIRES_WHEEL, hrd->input->relbit);
> +	__set_bit(EV_SW, hrd->input->evbit);
> +	__set_bit(SW_RATCHET, hrd->input->swbit);
> +
> +	/* Report current state of the ratchet switch */
> +	input_report_switch(hrd->input, SW_RATCHET, hrd->ratchet);
>  }
>  
>  
> -- 
> 2.9.3
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [v3,1/4] input: add an EV_REL event for high-res vertical wheel
  2017-04-07 15:10       ` Benjamin Tissoires
@ 2017-04-11 10:17         ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2017-04-11 10:17 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: linux-input, Dmitry Torokhov, Jonathan Corbet, Peter Hutterer,
	Hans Verkuil, Ping Cheng, Kamil Debski, Douglas Anderson,
	linux-doc

Em Fri, 7 Apr 2017 17:10:27 +0200
Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:

> On Apr 07 2017 or thereabouts, Mauro Carvalho Chehab wrote:
> > Em Fri, 7 Apr 2017 14:17:51 +0200
> > Benjamin Tissoires <benjamin.tissoires@redhat.com> escreveu:
> >   
> > > Hi Mauro,
> > > 
> > > On Apr 07 2017 or thereabouts, Mauro Carvalho Chehab wrote:  
> > > > As some devices can produce either low-res or high-res
> > > > vertical wheel EV_REL events, add a new event to allow
> > > > userspace to distinguish between them.
> > > > 
> > > > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > > > ---
> > > >  Documentation/input/event-codes.rst    | 16 +++++++++++++---
> > > >  include/uapi/linux/input-event-codes.h |  1 +
> > > >  2 files changed, 14 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
> > > > index 92db50954169..0c8591d39bc6 100644
> > > > --- a/Documentation/input/event-codes.rst
> > > > +++ b/Documentation/input/event-codes.rst
> > > > @@ -185,10 +185,20 @@ instead of EV_REL codes.
> > > >  
> > > >  A few EV_REL codes have special meanings:
> > > >  
> > > > -* REL_WHEEL, REL_HWHEEL:
> > > > +* REL_WHEEL:
> > > >  
> > > > -  - These codes are used for vertical and horizontal scroll wheels,
> > > > -    respectively.
> > > > +  - These codes are used for vertical scroll wheels.
> > > > +
> > > > +  - REL_WHEEL is for normal wheel operational mode, e. g. low-resolution
> > > > +    (line-based) scroll.
> > > > +
> > > > +  - REL_HIRES_WHEEL should be used when the wheel has two resolutions and it
> > > > +    is in high-resolution mode, e. g. the same angular movement that would
> > > > +    produce a single REL_WHEEL will produce multiple REL_HIRES_WHEEL events.
> > > > +
> > > > +* REL_HWHEEL:
> > > > +
> > > > +  - This code is used for horizontal scroll wheels.
> > > >  
> > > >  EV_ABS
> > > >  ------
> > > > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > > > index 3af60ee69053..23b2d377af59 100644
> > > > --- a/include/uapi/linux/input-event-codes.h
> > > > +++ b/include/uapi/linux/input-event-codes.h
> > > > @@ -703,6 +703,7 @@
> > > >  #define REL_DIAL		0x07
> > > >  #define REL_WHEEL		0x08
> > > >  #define REL_MISC		0x09
> > > > +#define REL_HIRES_WHEEL		0x0a    
> > > 
> > > There is one issue here. The HID input layer has a tendency to map
> > > usages to REL_MISC +1, +2, +3, etc... When it doesn't know how to map an
> > > usage, the core layer maps it to the next one.
> > > 
> > > I am not aware of devices exposing such REL axes, but I wouldn't be
> > > surprised if the issue will not raise at some point.
> > > 
> > > We already had an issue with ABS events where ABS_MISC are conflicting
> > > with ABS_MT_SLOTS. Luckily, we have a little empty space between
> > > ABS_MISC and ABS_MT_SLOT, which allows userspace to detect such issues.
> > > 
> > > Could you give the define a bigger number (and maybe reserve a couple of
> > > REL_MISC-n) to make sure that we do not end up with joysticks exporting
> > > REL_HIRES_WHEEL?  
> > 
> > Sure. How many REL_MISC-n should be reserved?  
> 
> Not sure. As I mentioned, I haven' t seen such device myself, but maybe
> we should reserve the rest of the 0x0n range for those (0x0a-0x0f). This
> would force to bump REL_MAX, but I don't think this is an issue besides
> recompiling libevdev and others.

Ok, I'll do that on a separate patch.

Btw, you requested to rebase patches on the top of HID tree.

Well, if I rebase on the top of HID for_next branch, patches 1 and 2
will have (trivial) conflicts with input-next, due to the documentation
patches that got merged there.

I might rebase just patches 3 and 4, but, in this case, the HID
branch won't compile.

So, I guess either input should pull from HID or vice-versa, in order
to avoid such conflicts.

> 
> >   
> > > I'll try the rest of the series on the MX Master today.  
> > 
> > It would be great if you could test it!  
> 
> I found some issues, I'll report in the other patches.

Ok.

> 
> Cheers,
> Benjamin
> 
> > 
> > Regards,
> > Mauro  



Thanks,
Mauro

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-04-11 10:17 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-07 11:31 [PATCH v3 0/4] add support for high res wheel found on some Logitech devices Mauro Carvalho Chehab
2017-04-07 11:31 ` [PATCH v3 1/4] input: add an EV_REL event for high-res vertical wheel Mauro Carvalho Chehab
2017-04-07 11:31   ` [PATCH v3 2/4] input: add a EV_SW event for ratchet switch Mauro Carvalho Chehab
2017-04-07 11:31     ` [PATCH v3 3/4] hid-logitech-hidpp: add support for high res wheel Mauro Carvalho Chehab
2017-04-07 11:31       ` [PATCH v3 4/4] hid-logitech-hidpp: add support for ratchet switch Mauro Carvalho Chehab
2017-04-07 15:33         ` Benjamin Tissoires
2017-04-07 15:31       ` [PATCH v3 3/4] hid-logitech-hidpp: add support for high res wheel Benjamin Tissoires
2017-04-07 12:17   ` [v3,1/4] input: add an EV_REL event for high-res vertical wheel Benjamin Tissoires
2017-04-07 13:32     ` Mauro Carvalho Chehab
2017-04-07 15:10       ` Benjamin Tissoires
2017-04-11 10:17         ` Mauro Carvalho Chehab

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.