Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 06/13] HID: logitech: move dj devices to the HID++ module
From: Benjamin Tissoires @ 2014-09-30 17:18 UTC (permalink / raw)
  To: Jiri Kosina, Nestor Lopez Casado, Andrew de los Reyes
  Cc: linux-kernel, linux-input
In-Reply-To: <1412097515-17241-1-git-send-email-benjamin.tissoires@redhat.com>

Devices connected through the Logitech Wireless Receiver are HID++ devices.
We can handle them here to benefit from this new module and activate
enhaced support of the various wireless touchpad or mice with touch
sensors on them.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/Kconfig              |  1 +
 drivers/hid/hid-logitech-dj.c    | 42 +---------------------------------------
 drivers/hid/hid-logitech-hidpp.c | 21 ++++++++++++--------
 3 files changed, 15 insertions(+), 49 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 6f299cd..dd49a9b 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -371,6 +371,7 @@ config HID_LOGITECH_DJ
 	tristate "Logitech Unifying receivers full support"
 	depends on HIDRAW
 	depends on HID_LOGITECH
+	select HID_LOGITECH_HIDPP
 	---help---
 	Say Y if you want support for Logitech Unifying receivers and devices.
 	Unifying receivers are capable of pairing up to 6 Logitech compliant
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 6aea16d..45a7eac 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -977,48 +977,8 @@ static struct hid_driver logi_djreceiver_driver = {
 #endif
 };
 
+module_hid_driver(logi_djreceiver_driver);
 
-static const struct hid_device_id logi_dj_devices[] = {
-	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
-		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
-	{}
-};
-
-static struct hid_driver logi_djdevice_driver = {
-	.name = "logitech-djdevice",
-	.id_table = logi_dj_devices,
-};
-
-
-static int __init logi_dj_init(void)
-{
-	int retval;
-
-	dbg_hid("Logitech-DJ:%s\n", __func__);
-
-	retval = hid_register_driver(&logi_djreceiver_driver);
-	if (retval)
-		return retval;
-
-	retval = hid_register_driver(&logi_djdevice_driver);
-	if (retval)
-		hid_unregister_driver(&logi_djreceiver_driver);
-
-	return retval;
-
-}
-
-static void __exit logi_dj_exit(void)
-{
-	dbg_hid("Logitech-DJ:%s\n", __func__);
-
-	hid_unregister_driver(&logi_djdevice_driver);
-	hid_unregister_driver(&logi_djreceiver_driver);
-
-}
-
-module_init(logi_dj_init);
-module_exit(logi_dj_exit);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Logitech");
 MODULE_AUTHOR("Nestor Lopez Casado");
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 7dd9163..48dec39 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -777,15 +777,17 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	hid_device_io_start(hdev);
 
 	connected = hidpp_is_connected(hidpp);
-	if (!connected) {
-		hid_err(hdev, "Device not connected");
-		goto hid_parse_fail;
-	}
+	if (id->group != HID_GROUP_LOGITECH_DJ_DEVICE) {
+		if (!connected) {
+			hid_err(hdev, "Device not connected");
+			goto hid_parse_fail;
+		}
 
-	/* the device is connected, we can ask for its name */
-	hid_info(hdev, "HID++ %u.%u device connected.\n",
-		 hidpp->protocol_major, hidpp->protocol_minor);
-	hidpp_overwrite_name(hdev);
+		/* the device is connected, we can ask for its name */
+		hid_info(hdev, "HID++ %u.%u device connected.\n",
+			 hidpp->protocol_major, hidpp->protocol_minor);
+		hidpp_overwrite_name(hdev);
+	}
 
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
 		ret = wtp_get_config(hidpp);
@@ -824,6 +826,9 @@ static const struct hid_device_id hidpp_devices[] = {
 	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH,
 		USB_DEVICE_ID_LOGITECH_T651),
 	  .driver_data = HIDPP_QUIRK_CLASS_WTP },
+
+	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
+		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
 	{}
 };
 
-- 
2.1.0

^ permalink raw reply related

* [PATCH 07/13] HID: logitech-dj: allow transfer of HID++ reports from/to the correct dj device
From: Benjamin Tissoires @ 2014-09-30 17:18 UTC (permalink / raw)
  To: Jiri Kosina, Nestor Lopez Casado, Andrew de los Reyes
  Cc: linux-kernel, linux-input
In-Reply-To: <1412097515-17241-1-git-send-email-benjamin.tissoires@redhat.com>

HID++ is a Logitech-specific protocol for communicating with HID
devices. DJ devices implement HID++, and so we can add the HID++
collection in the report descriptor and forward the incoming
reports from the receiver to the appropriate DJ device.

The same can be done in the other way, if someone calls a
.raw_request(), we can forward it to the correct dj device
by overriding the device_index in the HID++ report.

Signed-off-by: Benjamin Tisssoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-logitech-dj.c | 188 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 160 insertions(+), 28 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 45a7eac..feddacd 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -42,6 +42,15 @@
 #define REPORT_ID_DJ_SHORT			0x20
 #define REPORT_ID_DJ_LONG			0x21
 
+#define REPORT_ID_HIDPP_SHORT			0x10
+#define REPORT_ID_HIDPP_LONG			0x11
+
+#define HIDPP_REPORT_SHORT_LENGTH		7
+#define HIDPP_REPORT_LONG_LENGTH		20
+
+#define HIDPP_RECEIVER_INDEX			0xff
+
+#define REPORT_TYPE_RFREPORT_FIRST		0x01
 #define REPORT_TYPE_RFREPORT_LAST		0x1F
 
 /* Command Switch to DJ mode */
@@ -242,6 +251,57 @@ static const char media_descriptor[] = {
 	0xc0,			/* EndCollection                       */
 };				/*                                     */
 
+/* HIDPP descriptor */
+static const char hidpp_descriptor[] = {
+	0x06, 0x00, 0xff,	/* Usage Page (Vendor Defined Page 1)  */
+	0x09, 0x01,		/* Usage (Vendor Usage 1)              */
+	0xa1, 0x01,		/* Collection (Application)            */
+	0x85, 0x10,		/*   Report ID (16)                    */
+	0x75, 0x08,		/*   Report Size (8)                   */
+	0x95, 0x06,		/*   Report Count (6)                  */
+	0x15, 0x00,		/*   Logical Minimum (0)               */
+	0x26, 0xff, 0x00,	/*   Logical Maximum (255)             */
+	0x09, 0x01,		/*   Usage (Vendor Usage 1)            */
+	0x81, 0x00,		/*   Input (Data,Arr,Abs)              */
+	0x09, 0x01,		/*   Usage (Vendor Usage 1)            */
+	0x91, 0x00,		/*   Output (Data,Arr,Abs)             */
+	0xc0,			/* End Collection                      */
+	0x06, 0x00, 0xff,	/* Usage Page (Vendor Defined Page 1)  */
+	0x09, 0x02,		/* Usage (Vendor Usage 2)              */
+	0xa1, 0x01,		/* Collection (Application)            */
+	0x85, 0x11,		/*   Report ID (17)                    */
+	0x75, 0x08,		/*   Report Size (8)                   */
+	0x95, 0x13,		/*   Report Count (19)                 */
+	0x15, 0x00,		/*   Logical Minimum (0)               */
+	0x26, 0xff, 0x00,	/*   Logical Maximum (255)             */
+	0x09, 0x02,		/*   Usage (Vendor Usage 2)            */
+	0x81, 0x00,		/*   Input (Data,Arr,Abs)              */
+	0x09, 0x02,		/*   Usage (Vendor Usage 2)            */
+	0x91, 0x00,		/*   Output (Data,Arr,Abs)             */
+	0xc0,			/* End Collection                      */
+	0x06, 0x00, 0xff,	/* Usage Page (Vendor Defined Page 1)  */
+	0x09, 0x04,		/* Usage (Vendor Usage 0x04)           */
+	0xa1, 0x01,		/* Collection (Application)            */
+	0x85, 0x20,		/*   Report ID (32)                    */
+	0x75, 0x08,		/*   Report Size (8)                   */
+	0x95, 0x0e,		/*   Report Count (14)                 */
+	0x15, 0x00,		/*   Logical Minimum (0)               */
+	0x26, 0xff, 0x00,	/*   Logical Maximum (255)             */
+	0x09, 0x41,		/*   Usage (Vendor Usage 0x41)         */
+	0x81, 0x00,		/*   Input (Data,Arr,Abs)              */
+	0x09, 0x41,		/*   Usage (Vendor Usage 0x41)         */
+	0x91, 0x00,		/*   Output (Data,Arr,Abs)             */
+	0x85, 0x21,		/*   Report ID (33)                    */
+	0x95, 0x1f,		/*   Report Count (31)                 */
+	0x15, 0x00,		/*   Logical Minimum (0)               */
+	0x26, 0xff, 0x00,	/*   Logical Maximum (255)             */
+	0x09, 0x42,		/*   Usage (Vendor Usage 0x42)         */
+	0x81, 0x00,		/*   Input (Data,Arr,Abs)              */
+	0x09, 0x42,		/*   Usage (Vendor Usage 0x42)         */
+	0x91, 0x00,		/*   Output (Data,Arr,Abs)             */
+	0xc0,			/* End Collection                      */
+};
+
 /* Maximum size of all defined hid reports in bytes (including report id) */
 #define MAX_REPORT_SIZE 8
 
@@ -251,7 +311,8 @@ static const char media_descriptor[] = {
 	 sizeof(mse_descriptor) +		\
 	 sizeof(consumer_descriptor) +		\
 	 sizeof(syscontrol_descriptor) +	\
-	 sizeof(media_descriptor))
+	 sizeof(media_descriptor) +	\
+	 sizeof(hidpp_descriptor))
 
 /* Number of possible hid report types that can be created by this driver.
  *
@@ -512,6 +573,13 @@ static void logi_dj_recv_forward_report(struct dj_receiver_dev *djrcv_dev,
 	}
 }
 
+static void logi_dj_recv_forward_hidpp(struct dj_device *dj_dev, u8 *data,
+				       int size)
+{
+	/* We are called from atomic context (tasklet && djrcv->lock held) */
+	if (hid_input_report(dj_dev->hdev, HID_INPUT_REPORT, data, size, 1))
+		dbg_hid("hid_input_report error\n");
+}
 
 static int logi_dj_recv_send_report(struct dj_receiver_dev *djrcv_dev,
 				    struct dj_report *dj_report)
@@ -609,6 +677,16 @@ static int logi_dj_ll_raw_request(struct hid_device *hid,
 	u8 *out_buf;
 	int ret;
 
+	if ((buf[0] == REPORT_ID_HIDPP_SHORT) ||
+	    (buf[0] == REPORT_ID_HIDPP_LONG)) {
+		if (count < 2)
+			return -EINVAL;
+
+		buf[1] = djdev->device_index;
+		return hid_hw_raw_request(djrcv_dev->hdev, reportnum, buf,
+				count, report_type, reqtype);
+	}
+
 	if (buf[0] != REPORT_TYPE_LEDS)
 		return -EINVAL;
 
@@ -687,6 +765,8 @@ static int logi_dj_ll_parse(struct hid_device *hid)
 			__func__, djdev->reports_supported);
 	}
 
+	rdcat(rdesc, &rsize, hidpp_descriptor, sizeof(hidpp_descriptor));
+
 	retval = hid_parse_report(hid, rdesc, rsize);
 	kfree(rdesc);
 
@@ -714,8 +794,7 @@ static struct hid_ll_driver logi_dj_ll_driver = {
 	.raw_request = logi_dj_ll_raw_request,
 };
 
-
-static int logi_dj_raw_event(struct hid_device *hdev,
+static int logi_dj_dj_event(struct hid_device *hdev,
 			     struct hid_report *report, u8 *data,
 			     int size)
 {
@@ -723,36 +802,24 @@ static int logi_dj_raw_event(struct hid_device *hdev,
 	struct dj_report *dj_report = (struct dj_report *) data;
 	unsigned long flags;
 
-	dbg_hid("%s, size:%d\n", __func__, size);
-
-	/* Here we receive all data coming from iface 2, there are 4 cases:
-	 *
-	 * 1) Data should continue its normal processing i.e. data does not
-	 * come from the DJ collection, in which case we do nothing and
-	 * return 0, so hid-core can continue normal processing (will forward
-	 * to associated hidraw device)
+	/*
+	 * Here we receive all data coming from iface 2, there are 3 cases:
 	 *
-	 * 2) Data is from DJ collection, and is intended for this driver i. e.
-	 * data contains arrival, departure, etc notifications, in which case
-	 * we queue them for delayed processing by the work queue. We return 1
-	 * to hid-core as no further processing is required from it.
+	 * 1) Data is intended for this driver i. e. data contains arrival,
+	 * departure, etc notifications, in which case we queue them for delayed
+	 * processing by the work queue. We return 1 to hid-core as no further
+	 * processing is required from it.
 	 *
-	 * 3) Data is from DJ collection, and informs a connection change,
-	 * if the change means rf link loss, then we must send a null report
-	 * to the upper layer to discard potentially pressed keys that may be
-	 * repeated forever by the input layer. Return 1 to hid-core as no
-	 * further processing is required.
+	 * 2) Data informs a connection change, if the change means rf link
+	 * loss, then we must send a null report to the upper layer to discard
+	 * potentially pressed keys that may be repeated forever by the input
+	 * layer. Return 1 to hid-core as no further processing is required.
 	 *
-	 * 4) Data is from DJ collection and is an actual input event from
-	 * a paired DJ device in which case we forward it to the correct hid
-	 * device (via hid_input_report() ) and return 1 so hid-core does not do
-	 * anything else with it.
+	 * 3) Data is an actual input event from a paired DJ device in which
+	 * case we forward it to the correct hid device (via hid_input_report()
+	 * ) and return 1 so hid-core does not anything else with it.
 	 */
 
-	/* case 1) */
-	if (data[0] != REPORT_ID_DJ_SHORT)
-		return false;
-
 	if ((dj_report->device_index < DJ_DEVICE_INDEX_MIN) ||
 	    (dj_report->device_index > DJ_DEVICE_INDEX_MAX)) {
 		/*
@@ -797,6 +864,71 @@ out:
 	return true;
 }
 
+static int logi_dj_hidpp_event(struct hid_device *hdev,
+			     struct hid_report *report, u8 *data,
+			     int size)
+{
+	struct dj_receiver_dev *djrcv_dev = hid_get_drvdata(hdev);
+	struct dj_report *dj_report = (struct dj_report *) data;
+	unsigned long flags;
+	u8 device_index = dj_report->device_index;
+
+	if (device_index == HIDPP_RECEIVER_INDEX)
+		return false;
+
+	/*
+	 * Data is from the HID++ collection, in this case, we forward the
+	 * data to the corresponding child dj device and return 0 to hid-core
+	 * so he data also goes to the hidraw device of the receiver. This
+	 * allows a user space application to implement the full HID++ routing
+	 * via the receiver.
+	 */
+
+	if ((device_index < DJ_DEVICE_INDEX_MIN) ||
+	    (device_index > DJ_DEVICE_INDEX_MAX)) {
+		/*
+		 * Device index is wrong, bail out.
+		 * This driver can ignore safely the receiver notifications,
+		 * so ignore those reports too.
+		 */
+		dev_err(&hdev->dev, "%s: invalid device index:%d\n",
+				__func__, dj_report->device_index);
+		return false;
+	}
+
+	spin_lock_irqsave(&djrcv_dev->lock, flags);
+
+	if (!djrcv_dev->paired_dj_devices[device_index])
+		/* received an event for an unknown device, bail out */
+		goto out;
+
+	logi_dj_recv_forward_hidpp(djrcv_dev->paired_dj_devices[device_index],
+				   data, size);
+
+out:
+	spin_unlock_irqrestore(&djrcv_dev->lock, flags);
+
+	return false;
+}
+
+static int logi_dj_raw_event(struct hid_device *hdev,
+			     struct hid_report *report, u8 *data,
+			     int size)
+{
+	dbg_hid("%s, size:%d\n", __func__, size);
+
+	switch (data[0]) {
+	case REPORT_ID_DJ_SHORT:
+		return logi_dj_dj_event(hdev, report, data, size);
+	case REPORT_ID_HIDPP_SHORT:
+		/* intentional fallthrough */
+	case REPORT_ID_HIDPP_LONG:
+		return logi_dj_hidpp_event(hdev, report, data, size);
+	}
+
+	return false;
+}
+
 static int logi_dj_probe(struct hid_device *hdev,
 			 const struct hid_device_id *id)
 {
-- 
2.1.0

^ permalink raw reply related

* [PATCH 08/13] HID: logitech: allow the DJ device to request the unifying name
From: Benjamin Tissoires @ 2014-09-30 17:18 UTC (permalink / raw)
  To: Jiri Kosina, Nestor Lopez Casado, Andrew de los Reyes
  Cc: linux-kernel, linux-input
In-Reply-To: <1412097515-17241-1-git-send-email-benjamin.tissoires@redhat.com>

The names of the DJ devices are stored in the receiver. These names
can be retrieved through a HID++ command. However, the protocol says
that you have to ask the receiver for that, not the device iteself.

Introduce a special case in the DJ handling where a device can request
its unifying name, and when such a name is given, forward it also to
the corresponding device.

On the HID++ side, the receiver talks only HID++ 1.0, so we need to
implement this part of the protocol in the module.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-logitech-dj.c    | 24 ++++++++++--
 drivers/hid/hid-logitech-hidpp.c | 80 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 97 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index feddacd..9bc3942 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -667,6 +667,9 @@ static void logi_dj_ll_close(struct hid_device *hid)
 	dbg_hid("%s:%s\n", __func__, hid->phys);
 }
 
+static u8 unifying_name_query[]  = {0x10, 0xff, 0x83, 0xb5, 0x40, 0x00, 0x00};
+static u8 unifying_name_answer[] = {0x11, 0xff, 0x83, 0xb5};
+
 static int logi_dj_ll_raw_request(struct hid_device *hid,
 				  unsigned char reportnum, __u8 *buf,
 				  size_t count, unsigned char report_type,
@@ -682,7 +685,13 @@ static int logi_dj_ll_raw_request(struct hid_device *hid,
 		if (count < 2)
 			return -EINVAL;
 
-		buf[1] = djdev->device_index;
+		/* special case where we should not overwrite
+		 * the device_index */
+		if (count == 7 && !memcmp(buf, unifying_name_query,
+					  sizeof(unifying_name_query)))
+			buf[4] |= djdev->device_index - 1;
+		else
+			buf[1] = djdev->device_index;
 		return hid_hw_raw_request(djrcv_dev->hdev, reportnum, buf,
 				count, report_type, reqtype);
 	}
@@ -873,8 +882,17 @@ static int logi_dj_hidpp_event(struct hid_device *hdev,
 	unsigned long flags;
 	u8 device_index = dj_report->device_index;
 
-	if (device_index == HIDPP_RECEIVER_INDEX)
-		return false;
+	if (device_index == HIDPP_RECEIVER_INDEX) {
+		/* special case were the device wants to know its unifying
+		 * name */
+		if (size == HIDPP_REPORT_LONG_LENGTH &&
+		    !memcmp(data, unifying_name_answer,
+			    sizeof(unifying_name_answer)) &&
+		    ((data[4] & 0xF0) == 0x40))
+			device_index = (data[4] & 0x0F) + 1;
+		else
+			return false;
+	}
 
 	/*
 	 * Data is from the HID++ collection, in this case, we forward the
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 48dec39..e748e45 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -205,6 +205,31 @@ static int hidpp_send_fap_command_sync(struct hidpp_device *hidpp,
 	return ret;
 }
 
+static int hidpp_send_rap_command_sync(struct hidpp_device *hidpp_dev,
+	u8 report_id, u8 sub_id, u8 reg_address, u8 *params, int param_count,
+	struct hidpp_report *response)
+{
+	struct hidpp_report *message = kzalloc(sizeof(struct hidpp_report),
+			GFP_KERNEL);
+	int ret;
+
+	if ((report_id != REPORT_ID_HIDPP_SHORT) &&
+	    (report_id != REPORT_ID_HIDPP_LONG))
+		return -EINVAL;
+
+	if (param_count > sizeof(message->rap.params))
+		return -EINVAL;
+
+	message->report_id = report_id;
+	message->rap.sub_id = sub_id;
+	message->rap.reg_address = reg_address;
+	memcpy(&message->rap.params, params, param_count);
+
+	ret = hidpp_send_message_sync(hidpp_dev, message, response);
+	kfree(message);
+	return ret;
+}
+
 static inline bool hidpp_match_answer(struct hidpp_report *question,
 		struct hidpp_report *answer)
 {
@@ -221,6 +246,45 @@ static inline bool hidpp_match_error(struct hidpp_report *question,
 }
 
 /* -------------------------------------------------------------------------- */
+/* HIDP++ 1.0 commands                                                        */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_SET_REGISTER				0x80
+#define HIDPP_GET_REGISTER				0x81
+#define HIDPP_SET_LONG_REGISTER				0x82
+#define HIDPP_GET_LONG_REGISTER				0x83
+
+#define HIDPP_REG_PAIRING_INFORMATION			0xB5
+#define DEVICE_NAME					0x40
+
+static char *hidpp_get_unifying_name(struct hidpp_device *hidpp_dev)
+{
+	struct hidpp_report response;
+	int ret;
+	/* hid-logitech-dj is in charge of setting the right device index */
+	u8 params[1] = { DEVICE_NAME };
+	char *name;
+	int len;
+
+	ret = hidpp_send_rap_command_sync(hidpp_dev,
+					REPORT_ID_HIDPP_SHORT,
+					HIDPP_GET_LONG_REGISTER,
+					HIDPP_REG_PAIRING_INFORMATION,
+					params, 1, &response);
+	if (ret)
+		return NULL;
+
+	len = response.rap.params[1];
+
+	name = kzalloc(len + 1, GFP_KERNEL);
+	if (!name)
+		return NULL;
+
+	memcpy(name, &response.rap.params[2], len);
+	return name;
+}
+
+/* -------------------------------------------------------------------------- */
 /* 0x0000: Root                                                               */
 /* -------------------------------------------------------------------------- */
 
@@ -726,13 +790,21 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
 	return 0;
 }
 
-static void hidpp_overwrite_name(struct hid_device *hdev)
+static void hidpp_overwrite_name(struct hid_device *hdev, bool use_unifying)
 {
 	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
 	char *name;
 	u8 name_length;
 
-	name = hidpp_get_device_name(hidpp, &name_length);
+	if (use_unifying)
+		/*
+		 * the device is connected through an Unifying receiver, and
+		 * might not be already connected.
+		 * Ask the receiver for its name.
+		 */
+		name = hidpp_get_unifying_name(hidpp);
+	else
+		name = hidpp_get_device_name(hidpp, &name_length);
 
 	if (!name)
 		hid_err(hdev, "unable to retrieve the name of the device");
@@ -783,12 +855,12 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 			goto hid_parse_fail;
 		}
 
-		/* the device is connected, we can ask for its name */
 		hid_info(hdev, "HID++ %u.%u device connected.\n",
 			 hidpp->protocol_major, hidpp->protocol_minor);
-		hidpp_overwrite_name(hdev);
 	}
 
+	hidpp_overwrite_name(hdev, id->group == HID_GROUP_LOGITECH_DJ_DEVICE);
+
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
 		ret = wtp_get_config(hidpp);
 		if (ret)
-- 
2.1.0

^ permalink raw reply related

* [PATCH 09/13] HID: logitech-dj: enable notifications on connect/disconnect
From: Benjamin Tissoires @ 2014-09-30 17:18 UTC (permalink / raw)
  To: Jiri Kosina, Nestor Lopez Casado, Andrew de los Reyes
  Cc: linux-kernel, linux-input
In-Reply-To: <1412097515-17241-1-git-send-email-benjamin.tissoires@redhat.com>

The receiver can send HID++ notifications to the DJ devices when the
physical devices are connected/disconnected.
Enable this feature by default.

This command uses a HID++ command instead of a DJ one, so use a direct
call to usbhid instead of using logi_dj_recv_send_report()

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-logitech-dj.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 9bc3942..c917ab6 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -630,7 +630,9 @@ static int logi_dj_recv_query_paired_devices(struct dj_receiver_dev *djrcv_dev)
 static int logi_dj_recv_switch_to_dj_mode(struct dj_receiver_dev *djrcv_dev,
 					  unsigned timeout)
 {
+	struct hid_device *hdev = djrcv_dev->hdev;
 	struct dj_report *dj_report;
+	u8 *buf;
 	int retval;
 
 	dj_report = kzalloc(sizeof(struct dj_report), GFP_KERNEL);
@@ -642,7 +644,6 @@ static int logi_dj_recv_switch_to_dj_mode(struct dj_receiver_dev *djrcv_dev,
 	dj_report->report_params[CMD_SWITCH_PARAM_DEVBITFIELD] = 0x3F;
 	dj_report->report_params[CMD_SWITCH_PARAM_TIMEOUT_SECONDS] = (u8)timeout;
 	retval = logi_dj_recv_send_report(djrcv_dev, dj_report);
-	kfree(dj_report);
 
 	/*
 	 * Ugly sleep to work around a USB 3.0 bug when the receiver is still
@@ -651,6 +652,30 @@ static int logi_dj_recv_switch_to_dj_mode(struct dj_receiver_dev *djrcv_dev,
 	 */
 	msleep(50);
 
+	/*
+	 * Magical bits to set up hidpp notifications when the dj devices
+	 * are connected/disconnected.
+	 *
+	 * We can reuse dj_report because HIDPP_REPORT_SHORT_LENGTH is smaller
+	 * than DJREPORT_SHORT_LENGTH.
+	 */
+	buf = (u8 *)dj_report;
+
+	memset(buf, 0, HIDPP_REPORT_SHORT_LENGTH);
+
+	buf[0] = REPORT_ID_HIDPP_SHORT;
+	buf[1] = 0xFF;
+	buf[2] = 0x80;
+	buf[3] = 0x00;
+	buf[4] = 0x00;
+	buf[5] = 0x09;
+	buf[6] = 0x00;
+
+	hid_hw_raw_request(hdev, REPORT_ID_HIDPP_SHORT, buf,
+			HIDPP_REPORT_SHORT_LENGTH, HID_OUTPUT_REPORT,
+			HID_REQ_SET_REPORT);
+
+	kfree(dj_report);
 	return retval;
 }
 
-- 
2.1.0

^ permalink raw reply related

* [PATCH 10/13] HID: logitech-hidpp: late bind the input device on wireless connection
From: Benjamin Tissoires @ 2014-09-30 17:18 UTC (permalink / raw)
  To: Jiri Kosina, Nestor Lopez Casado, Andrew de los Reyes
  Cc: linux-kernel, linux-input
In-Reply-To: <1412097515-17241-1-git-send-email-benjamin.tissoires@redhat.com>

Now that the receiver forwards the connect/disconnect events, we can
know when the device is available to communicate with us.

When it is ready, we can for instance retrieve its full name, which
guarantee that we always have the same name for the DJ device (the DJ
name is somewhat shorter than the HID++ name).

This mechanism is mandatory for the touchpads line, which has the
min/max information stored in the device. This information can only
be retrieved when the device is connected. So we can not populate
the input device until we are sure that the device is connected.

This patch creates a new input device for such devices. However,
this input is not bound to hid directly, so the various drivers
which wants to use it are required to process completely the
incoming reports in .raw_event().

Note that the patch in itself just adds the bits for the next
ones, and this feature is disabled by default.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-logitech-hidpp.c | 155 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 147 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index e748e45..9561a1f 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -36,6 +36,9 @@ MODULE_AUTHOR("Nestor Lopez Casado <nlopezcasad@logitech.com>");
 
 #define HIDPP_QUIRK_CLASS_WTP			BIT(0)
 
+/* bits 1..20 are reserved for classes */
+#define HIDPP_QUIRK_DELAYED_INIT		BIT(21)
+
 /*
  * There are two hidpp protocols in use, the first version hidpp10 is known
  * as register access protocol or RAP, the second version hidpp20 is known as
@@ -91,6 +94,11 @@ struct hidpp_device {
 
 	void *private_data;
 
+	struct work_struct work;
+	struct kfifo delayed_work_fifo;
+	atomic_t connected;
+	struct input_dev *delayed_input;
+
 	unsigned long quirks;
 };
 
@@ -110,6 +118,8 @@ struct hidpp_device {
 #define HIDPP_ERROR_INVALID_PARAM_VALUE		0x0b
 #define HIDPP_ERROR_WRONG_PIN_CODE		0x0c
 
+static void hidpp_connect_event(struct hidpp_device *hidpp_dev);
+
 static int __hidpp_send_report(struct hid_device *hdev,
 				struct hidpp_report *hidpp_report)
 {
@@ -230,6 +240,13 @@ static int hidpp_send_rap_command_sync(struct hidpp_device *hidpp_dev,
 	return ret;
 }
 
+static void delayed_work_cb(struct work_struct *work)
+{
+	struct hidpp_device *hidpp = container_of(work, struct hidpp_device,
+							work);
+	hidpp_connect_event(hidpp);
+}
+
 static inline bool hidpp_match_answer(struct hidpp_report *question,
 		struct hidpp_report *answer)
 {
@@ -245,6 +262,12 @@ static inline bool hidpp_match_error(struct hidpp_report *question,
 	    (answer->fap.params[0] == question->fap.funcindex_clientid);
 }
 
+static inline bool hidpp_report_is_connect_event(struct hidpp_report *report)
+{
+	return (report->report_id == REPORT_ID_HIDPP_SHORT) &&
+		(report->rap.sub_id == 0x41);
+}
+
 /* -------------------------------------------------------------------------- */
 /* HIDP++ 1.0 commands                                                        */
 /* -------------------------------------------------------------------------- */
@@ -535,12 +558,10 @@ static int wtp_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	return -1;
 }
 
-static void wtp_input_configured(struct hid_device *hdev,
-				struct hid_input *hidinput)
+static void wtp_populate_input(struct hidpp_device *hidpp,
+		struct input_dev *input_dev, bool origin_is_hid_core)
 {
-	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
 	struct wtp_data *wd = hidpp->private_data;
-	struct input_dev *input_dev = hidinput->input;
 
 	__set_bit(EV_ABS, input_dev->evbit);
 	__set_bit(EV_KEY, input_dev->evbit);
@@ -716,13 +737,20 @@ static int hidpp_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	return 0;
 }
 
+static void hidpp_populate_input(struct hidpp_device *hidpp,
+		struct input_dev *input, bool origin_is_hid_core)
+{
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
+		wtp_populate_input(hidpp, input, origin_is_hid_core);
+}
+
 static void hidpp_input_configured(struct hid_device *hdev,
 				struct hid_input *hidinput)
 {
 	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+	struct input_dev *input = hidinput->input;
 
-	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
-		wtp_input_configured(hdev, hidinput);
+	hidpp_populate_input(hidpp, input, true);
 }
 
 static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
@@ -756,6 +784,15 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
 		}
 	}
 
+	if (unlikely(hidpp_report_is_connect_event(report))) {
+		atomic_set(&hidpp->connected,
+				!(report->rap.params[0] & (1 << 6)));
+		if ((hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT) &&
+		    (schedule_work(&hidpp->work) == 0))
+			dbg_hid("%s: connect event already queued\n", __func__);
+		return 1;
+	}
+
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		return wtp_raw_event(hidpp->hid_dev, data, size);
 
@@ -814,11 +851,99 @@ static void hidpp_overwrite_name(struct hid_device *hdev, bool use_unifying)
 	kfree(name);
 }
 
+static int hidpp_input_open(struct input_dev *dev)
+{
+	struct hid_device *hid = input_get_drvdata(dev);
+
+	return hid_hw_open(hid);
+}
+
+static void hidpp_input_close(struct input_dev *dev)
+{
+	struct hid_device *hid = input_get_drvdata(dev);
+
+	hid_hw_close(hid);
+}
+
+static struct input_dev *hidpp_allocate_input(struct hid_device *hdev)
+{
+	struct input_dev *input_dev = devm_input_allocate_device(&hdev->dev);
+
+	if (!input_dev)
+		return NULL;
+
+	input_set_drvdata(input_dev, hdev);
+	input_dev->open = hidpp_input_open;
+	input_dev->close = hidpp_input_close;
+
+	input_dev->name = hdev->name;
+	input_dev->phys = hdev->phys;
+	input_dev->uniq = hdev->uniq;
+	input_dev->id.bustype = hdev->bus;
+	input_dev->id.vendor  = hdev->vendor;
+	input_dev->id.product = hdev->product;
+	input_dev->id.version = hdev->version;
+	input_dev->dev.parent = &hdev->dev;
+
+	return input_dev;
+}
+
+static void hidpp_connect_event(struct hidpp_device *hidpp)
+{
+	struct hid_device *hdev = hidpp->hid_dev;
+	int ret = 0;
+	bool connected = atomic_read(&hidpp->connected);
+	struct input_dev *input;
+	char *name, *devm_name;
+	u8 name_length;
+
+	if (!connected || hidpp->delayed_input)
+		return;
+
+	if (!hidpp->protocol_major) {
+		ret = !hidpp_is_connected(hidpp);
+		if (ret) {
+			hid_err(hdev, "Can not get the protocol version.\n");
+			return;
+		}
+	}
+
+	/* the device is already connected, we can ask for its name and
+	 * protocol */
+	hid_info(hdev, "HID++ %u.%u device connected.\n",
+		 hidpp->protocol_major, hidpp->protocol_minor);
+
+	input = hidpp_allocate_input(hdev);
+	if (!input) {
+		hid_err(hdev, "cannot allocate new input device: %d\n", ret);
+		return;
+	}
+
+	name = hidpp_get_device_name(hidpp, &name_length);
+	if (!name) {
+		hid_err(hdev, "unable to retrieve the name of the device");
+	} else {
+		devm_name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "%s", name);
+		if (devm_name)
+			input->name = devm_name;
+		kfree(name);
+	}
+
+	hidpp_populate_input(hidpp, input, false);
+
+	ret = input_register_device(input);
+	if (ret)
+		input_free_device(input);
+
+	hidpp->delayed_input = input;
+}
+
 static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	struct hidpp_device *hidpp;
 	int ret;
 	bool connected;
+	unsigned int connect_mask = HID_CONNECT_DEFAULT;
 
 	hidpp = devm_kzalloc(&hdev->dev, sizeof(struct hidpp_device),
 			GFP_KERNEL);
@@ -836,6 +961,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 			return ret;
 	}
 
+	INIT_WORK(&hidpp->work, delayed_work_cb);
 	mutex_init(&hidpp->send_mutex);
 	init_waitqueue_head(&hidpp->wait);
 
@@ -860,8 +986,9 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	hidpp_overwrite_name(hdev, id->group == HID_GROUP_LOGITECH_DJ_DEVICE);
+	atomic_set(&hidpp->connected, connected);
 
-	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
+	if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) {
 		ret = wtp_get_config(hidpp);
 		if (ret)
 			goto hid_parse_fail;
@@ -870,16 +997,27 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	/* Block incoming packets */
 	hid_device_io_stop(hdev);
 
-	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
+		connect_mask &= ~HID_CONNECT_HIDINPUT;
+
+	ret = hid_hw_start(hdev, connect_mask);
 	if (ret) {
 		hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
 		goto hid_hw_start_fail;
 	}
 
+	if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT) {
+		/* Allow incoming packets */
+		hid_device_io_start(hdev);
+
+		hidpp_connect_event(hidpp);
+	}
+
 	return ret;
 
 hid_hw_start_fail:
 hid_parse_fail:
+	cancel_work_sync(&hidpp->work);
 	mutex_destroy(&hidpp->send_mutex);
 	hid_set_drvdata(hdev, NULL);
 	return ret;
@@ -889,6 +1027,7 @@ static void hidpp_remove(struct hid_device *hdev)
 {
 	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
 
+	cancel_work_sync(&hidpp->work);
 	mutex_destroy(&hidpp->send_mutex);
 	hid_hw_stop(hdev);
 }
-- 
2.1.0

^ permalink raw reply related

* [PATCH 11/13] HID: logitech-hidpp: Add Wireless Touchpad T650 support
From: Benjamin Tissoires @ 2014-09-30 17:18 UTC (permalink / raw)
  To: Jiri Kosina, Nestor Lopez Casado, Andrew de los Reyes
  Cc: linux-kernel, linux-input
In-Reply-To: <1412097515-17241-1-git-send-email-benjamin.tissoires@redhat.com>

All the bits are now in place to add the support of the
Touchpad T650.
The creation/population of the input device is delayed until
the device is ready.

The T650 uses the special HID++ reporting protocol, so activate
this on connect.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-logitech-hidpp.c | 105 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 103 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 9561a1f..f9a4ec0 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -471,6 +471,9 @@ out_err:
 #define HIDPP_PAGE_TOUCHPAD_RAW_XY			0x6100
 
 #define CMD_TOUCHPAD_GET_RAW_INFO			0x01
+#define CMD_TOUCHPAD_SET_RAW_REPORT_STATE		0x21
+
+#define EVENT_TOUCHPAD_RAW_XY				0x00
 
 #define TOUCHPAD_RAW_XY_ORIGIN_LOWER_LEFT		0x01
 #define TOUCHPAD_RAW_XY_ORIGIN_UPPER_LEFT		0x03
@@ -530,6 +533,59 @@ static int hidpp_touchpad_get_raw_info(struct hidpp_device *hidpp,
 	return ret;
 }
 
+static int hidpp_touchpad_set_raw_report_state(struct hidpp_device *hidpp_dev,
+		u8 feature_index, bool send_raw_reports,
+		bool sensor_enhanced_settings)
+{
+	struct hidpp_report response;
+
+	/*
+	 * Params:
+	 *   bit 0 - enable raw
+	 *   bit 1 - 16bit Z, no area
+	 *   bit 2 - enhanced sensitivity
+	 *   bit 3 - width, height (4 bits each) instead of area
+	 *   bit 4 - send raw + gestures (degrades smoothness)
+	 *   remaining bits - reserved
+	 */
+	u8 params = send_raw_reports | (sensor_enhanced_settings << 2);
+
+	return hidpp_send_fap_command_sync(hidpp_dev, feature_index,
+		CMD_TOUCHPAD_SET_RAW_REPORT_STATE, &params, 1, &response);
+}
+
+static void hidpp_touchpad_touch_event(u8 *data,
+	struct hidpp_touchpad_raw_xy_finger *finger)
+{
+	u8 x_m = data[0] << 2;
+	u8 y_m = data[2] << 2;
+
+	finger->x = x_m << 6 | data[1];
+	finger->y = y_m << 6 | data[3];
+
+	finger->contact_type = data[0] >> 6;
+	finger->contact_status = data[2] >> 6;
+
+	finger->z = data[4];
+	finger->area = data[5];
+	finger->finger_id = data[6] >> 4;
+}
+
+static void hidpp_touchpad_raw_xy_event(struct hidpp_device *hidpp_dev,
+		u8 *data, struct hidpp_touchpad_raw_xy *raw_xy)
+{
+	memset(raw_xy, 0, sizeof(struct hidpp_touchpad_raw_xy));
+	raw_xy->end_of_frame = data[8] & 0x01;
+	raw_xy->spurious_flag = (data[8] >> 1) & 0x01;
+	raw_xy->finger_count = data[15] & 0x0f;
+	raw_xy->button = (data[8] >> 2) & 0x01;
+
+	if (raw_xy->finger_count) {
+		hidpp_touchpad_touch_event(&data[2], &raw_xy->fingers[0]);
+		hidpp_touchpad_touch_event(&data[9], &raw_xy->fingers[1]);
+	}
+}
+
 /* ************************************************************************** */
 /*                                                                            */
 /* Device Support                                                             */
@@ -672,11 +728,28 @@ static int wtp_raw_event(struct hid_device *hdev, u8 *data, int size)
 {
 	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
 	struct wtp_data *wd = hidpp->private_data;
+	struct hidpp_report *report = (struct hidpp_report *)data;
+	struct hidpp_touchpad_raw_xy raw;
 
-	if (!wd || !wd->input || (data[0] != 0x02) || size < 21)
+	if (!wd || !wd->input)
 		return 1;
 
-	return wtp_mouse_raw_xy_event(hidpp, &data[7]);
+	switch (data[0]) {
+	case 0x02:
+		if (size < 21)
+			return 1;
+		return wtp_mouse_raw_xy_event(hidpp, &data[7]);
+	case REPORT_ID_HIDPP_LONG:
+		if ((report->fap.feature_index != wd->mt_feature_index) ||
+		    (report->fap.funcindex_clientid != EVENT_TOUCHPAD_RAW_XY))
+			return 1;
+		hidpp_touchpad_raw_xy_event(hidpp, data + 4, &raw);
+
+		wtp_send_raw_xy_event(hidpp, &raw);
+		return 0;
+	}
+
+	return 0;
 }
 
 static int wtp_get_config(struct hidpp_device *hidpp)
@@ -721,6 +794,27 @@ static int wtp_allocate(struct hid_device *hdev, const struct hid_device_id *id)
 	return 0;
 };
 
+static void wtp_connect(struct hid_device *hdev, bool connected)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+	struct wtp_data *wd = hidpp->private_data;
+	int ret;
+
+	if (!connected)
+		return;
+
+	if (!wd->x_size) {
+		ret = wtp_get_config(hidpp);
+		if (ret) {
+			hid_err(hdev, "Can not get wtp config: %d\n", ret);
+			return;
+		}
+	}
+
+	hidpp_touchpad_set_raw_report_state(hidpp, wd->mt_feature_index,
+			true, true);
+}
+
 /* -------------------------------------------------------------------------- */
 /* Generic HID++ devices                                                      */
 /* -------------------------------------------------------------------------- */
@@ -897,6 +991,9 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 	char *name, *devm_name;
 	u8 name_length;
 
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
+		wtp_connect(hdev, connected);
+
 	if (!connected || hidpp->delayed_input)
 		return;
 
@@ -1033,6 +1130,10 @@ static void hidpp_remove(struct hid_device *hdev)
 }
 
 static const struct hid_device_id hidpp_devices[] = {
+	{ /* wireless touchpad T650 */
+	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
+		USB_VENDOR_ID_LOGITECH, 0x4101),
+	  .driver_data = HIDPP_QUIRK_CLASS_WTP | HIDPP_QUIRK_DELAYED_INIT },
 	{ /* wireless touchpad T651 */
 	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH,
 		USB_DEVICE_ID_LOGITECH_T651),
-- 
2.1.0

^ permalink raw reply related

* [PATCH 12/13] HID: logitech-hidpp: add support of the first Logitech Wireless Touchpad
From: Benjamin Tissoires @ 2014-09-30 17:18 UTC (permalink / raw)
  To: Jiri Kosina, Nestor Lopez Casado, Andrew de los Reyes
  Cc: linux-kernel, linux-input
In-Reply-To: <1412097515-17241-1-git-send-email-benjamin.tissoires@redhat.com>

This touchpad differs from the T650 in several ways:
- the resolution is not correctly returned by the device
- it presents physical buttons, so the button flag in the raw touch report
  is not filled.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-logitech-hidpp.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index f9a4ec0..17e27e9 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -38,6 +38,7 @@ MODULE_AUTHOR("Nestor Lopez Casado <nlopezcasad@logitech.com>");
 
 /* bits 1..20 are reserved for classes */
 #define HIDPP_QUIRK_DELAYED_INIT		BIT(21)
+#define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS	BIT(22)
 
 /*
  * There are two hidpp protocols in use, the first version hidpp10 is known
@@ -596,6 +597,8 @@ static void hidpp_touchpad_raw_xy_event(struct hidpp_device *hidpp_dev,
 /* Touchpad HID++ devices                                                     */
 /* -------------------------------------------------------------------------- */
 
+#define WTP_MANUAL_RESOLUTION				39
+
 struct wtp_data {
 	struct input_dev *input;
 	u16 x_size, y_size;
@@ -634,7 +637,10 @@ static void wtp_populate_input(struct hidpp_device *hidpp,
 
 	input_set_capability(input_dev, EV_KEY, BTN_LEFT);
 
-	__set_bit(INPUT_PROP_BUTTONPAD, input_dev->propbit);
+	if (hidpp->quirks & HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS)
+		input_set_capability(input_dev, EV_KEY, BTN_RIGHT);
+	else
+		__set_bit(INPUT_PROP_BUTTONPAD, input_dev->propbit);
 
 	input_mt_init_slots(input_dev, wd->maxcontacts, INPUT_MT_POINTER |
 		INPUT_MT_DROP_UNUSED);
@@ -676,7 +682,8 @@ static void wtp_send_raw_xy_event(struct hidpp_device *hidpp,
 	for (i = 0; i < 2; i++)
 		wtp_touch_event(wd, &(raw->fingers[i]));
 
-	if (raw->end_of_frame)
+	if (raw->end_of_frame &&
+	    !(hidpp->quirks & HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS))
 		input_event(wd->input, EV_KEY, BTN_LEFT, raw->button);
 
 	if (raw->end_of_frame || raw->finger_count <= 2) {
@@ -736,9 +743,17 @@ static int wtp_raw_event(struct hid_device *hdev, u8 *data, int size)
 
 	switch (data[0]) {
 	case 0x02:
-		if (size < 21)
-			return 1;
-		return wtp_mouse_raw_xy_event(hidpp, &data[7]);
+		if (hidpp->quirks & HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS) {
+			input_event(wd->input, EV_KEY, BTN_LEFT,
+					!!(data[1] & 0x01));
+			input_event(wd->input, EV_KEY, BTN_RIGHT,
+					!!(data[1] & 0x02));
+			input_sync(wd->input);
+		} else {
+			if (size < 21)
+				return 1;
+			return wtp_mouse_raw_xy_event(hidpp, &data[7]);
+		}
 	case REPORT_ID_HIDPP_LONG:
 		if ((report->fap.feature_index != wd->mt_feature_index) ||
 		    (report->fap.funcindex_clientid != EVENT_TOUCHPAD_RAW_XY))
@@ -775,6 +790,8 @@ static int wtp_get_config(struct hidpp_device *hidpp)
 	wd->maxcontacts = raw_info.maxcontacts;
 	wd->flip_y = raw_info.origin == TOUCHPAD_RAW_XY_ORIGIN_LOWER_LEFT;
 	wd->resolution = raw_info.res;
+	if (!wd->resolution)
+		wd->resolution = WTP_MANUAL_RESOLUTION;
 
 	return 0;
 }
@@ -1130,6 +1147,11 @@ static void hidpp_remove(struct hid_device *hdev)
 }
 
 static const struct hid_device_id hidpp_devices[] = {
+	{ /* wireless touchpad */
+	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
+		USB_VENDOR_ID_LOGITECH, 0x4011),
+	  .driver_data = HIDPP_QUIRK_CLASS_WTP | HIDPP_QUIRK_DELAYED_INIT |
+			 HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS },
 	{ /* wireless touchpad T650 */
 	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, 0x4101),
-- 
2.1.0

^ permalink raw reply related

* [PATCH 13/13] HID: logitech-hidpp: support combo keyboard touchpad TK820
From: Benjamin Tissoires @ 2014-09-30 17:18 UTC (permalink / raw)
  To: Jiri Kosina, Nestor Lopez Casado, Andrew de los Reyes
  Cc: linux-kernel, linux-input
In-Reply-To: <1412097515-17241-1-git-send-email-benjamin.tissoires@redhat.com>

The TK820 presents both a keyboard and a touchpad on the same
physical (and logical device). Use the generic hid-input
processing for the keyboard part. The keyboard input device is created
when the receiver is plugged in, so no events are missed on connect.

When the device actaully connects, we can set it to use the raw
multitouch reporting to have a consistent user experience accross
all Logitech touchpads.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-logitech-hidpp.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 17e27e9..361e97d 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -39,6 +39,7 @@ MODULE_AUTHOR("Nestor Lopez Casado <nlopezcasad@logitech.com>");
 /* bits 1..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)
 
 /*
  * There are two hidpp protocols in use, the first version hidpp10 is known
@@ -614,6 +615,12 @@ static int wtp_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+
+	if ((hidpp->quirks & HIDPP_QUIRK_MULTI_INPUT) &&
+	    (field->application == HID_GD_KEYBOARD))
+		return 0;
+
 	return -1;
 }
 
@@ -622,6 +629,10 @@ static void wtp_populate_input(struct hidpp_device *hidpp,
 {
 	struct wtp_data *wd = hidpp->private_data;
 
+	if ((hidpp->quirks & HIDPP_QUIRK_MULTI_INPUT) && origin_is_hid_core)
+		/* this is the generic hid-input call */
+		return;
+
 	__set_bit(EV_ABS, input_dev->evbit);
 	__set_bit(EV_KEY, input_dev->evbit);
 	__clear_bit(EV_REL, input_dev->evbit);
@@ -1114,6 +1125,10 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
 		connect_mask &= ~HID_CONNECT_HIDINPUT;
 
+	/* Re-enable hidinput for multi-input devices */
+	if (hidpp->quirks & HIDPP_QUIRK_MULTI_INPUT)
+		connect_mask |= HID_CONNECT_HIDINPUT;
+
 	ret = hid_hw_start(hdev, connect_mask);
 	if (ret) {
 		hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
@@ -1160,6 +1175,11 @@ static const struct hid_device_id hidpp_devices[] = {
 	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH,
 		USB_DEVICE_ID_LOGITECH_T651),
 	  .driver_data = HIDPP_QUIRK_CLASS_WTP },
+	{ /* Keyboard TK820 */
+	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
+		USB_VENDOR_ID_LOGITECH, 0x4102),
+	  .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_MULTI_INPUT |
+			 HIDPP_QUIRK_CLASS_WTP },
 
 	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
-- 
2.1.0

^ permalink raw reply related

* [PATCH 00/13] HID: add support of Logitech touchpads and special devices
From: Benjamin Tissoires @ 2014-09-30 17:18 UTC (permalink / raw)
  To: Jiri Kosina, Nestor Lopez Casado, Andrew de los Reyes
  Cc: linux-kernel, linux-input

Hi,

Well this patch series is really an old guy:
- I started on this back in 2011 during my short appearance at Logitech
- it has been updated a few time since
- part of it is already shipped in ChromeOS
- now is the time to push it upstream (hopefully)

So, basically, this patch series aims at supporting the touchpads made by
Logitech (i.e. report the raw multi touch events), plus it also gives a driver
to enable special "features" of other various Logitech devices as reported
here[1].

There are couple of changes I introduced in this submission compared to what
Logitech and ChromeOS used to see:
- there will be only one driver for HID++ devices: hid-logitech-hidpp (this way,
  we are trying to stay in the line of the HID subsystem, one HID driver per
  vendor)
- hid-logitech-dj only deals with the transport layer, so it does not depend on
  hid-logitech-hidpp
- there is no more callbacks mechanism in hidpp and the specific drivers, all
  is handled in one place
- there is no need of a blacklist in hid-logitech-dj to decide if the device has
  a subdriver or not.
- there is no support of the mice with raw touch. This can be added in a future
  patch series.

I am finally finding this ready to be sent upstream, so I hope some of you would
be kind enough to review the code.

Regarding the features, here is what end user will expect/see:
- all the DJ devices (connected through the unifying wirelees receiver) have a
  proper name now (T400, M325, ...). This name is retrieved from the receiver
  which has a short buffer for it, thus the short name.
- the touchpads input devices will be created once the device is actually
  connected. For bluetooth device, it's a normal HID process. For DJ devices,
  the receiver is created, the hidraw device of the touchpad is created, but the
  input device, with the full name (asked from the device) appears on the first
  connection (or events).
- The TK820 - keyboard/touchpad combo presents now 2 input devices. One for the
  keyboard part, one for the touchpad. So X and libinput can deal with it
  easily.

Jiri, This patch series can be split in 2 if you would like:
- patches 1 to 4 can be scheduled easily for 3.18 IMO
- patches 5 to 13 might need some more reviews, so either 3.18 or 3.19

Cheers,
Benjamin

[1] https://www.mail-archive.com/linux-input@vger.kernel.org/msg11831.html

Benjamin Tissoires (13):
  HID: fix merge from wacom into the HID tree
  HID: core: do not scan reports if the group is already set
  HID: logitech-dj: rely on hid groups to separate receivers from dj
    devices
  HID: logitech-dj: merge header file into the source
  HID: Introduce hidpp, a module to handle Logitech hid++ devices
  HID: logitech: move dj devices to the HID++ module
  HID: logitech-dj: allow transfer of HID++ reports from/to the correct
    dj device
  HID: logitech: allow the DJ device to request the unifying name
  HID: logitech-dj: enable notifications on connect/disconnect
  HID: logitech-hidpp: late bind the input device on wireless connection
  HID: logitech-hidpp: Add Wireless Touchpad T650 support
  HID: logitech-hidpp: add support of the first Logitech Wireless
    Touchpad
  HID: logitech-hidpp: support combo keyboard touchpad TK820

 drivers/hid/Kconfig              |   12 +
 drivers/hid/Makefile             |    1 +
 drivers/hid/hid-core.c           |   21 +-
 drivers/hid/hid-ids.h            |    1 +
 drivers/hid/hid-logitech-dj.c    |  397 ++++++++++---
 drivers/hid/hid-logitech-dj.h    |  125 ----
 drivers/hid/hid-logitech-hidpp.c | 1201 ++++++++++++++++++++++++++++++++++++++
 include/linux/hid.h              |    5 +-
 8 files changed, 1526 insertions(+), 237 deletions(-)
 delete mode 100644 drivers/hid/hid-logitech-dj.h
 create mode 100644 drivers/hid/hid-logitech-hidpp.c

-- 
2.1.0


^ permalink raw reply

* [PATCH 03/13] HID: logitech-dj: rely on hid groups to separate receivers from dj devices
From: Benjamin Tissoires @ 2014-09-30 17:18 UTC (permalink / raw)
  To: Jiri Kosina, Nestor Lopez Casado, Andrew de los Reyes
  Cc: linux-kernel, linux-input
In-Reply-To: <1412097515-17241-1-git-send-email-benjamin.tissoires@redhat.com>

Several benefits here:
- we can drop the macro is_dj_device: I never been really conviced by
  this macro as we could fall into a null pointer anytime. Anyway time
  showed that this never happened.
- we can simplify the hid driver logitech-djdevice, and make it aware
  of any new receiver VID/PID.
- we can use the Wireless PID of the DJ device as the product id of the
  hid device, this way the sysfs will differentiate between different
  DJ devices.

Signed-off-by: Benjamin Tisssoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-logitech-dj.c | 38 ++++++++++----------------------------
 drivers/hid/hid-logitech-dj.h | 10 ----------
 include/linux/hid.h           |  1 +
 3 files changed, 11 insertions(+), 38 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 71f5692..42d38d5 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -256,11 +256,15 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
 	dj_hiddev->dev.parent = &djrcv_hdev->dev;
 	dj_hiddev->bus = BUS_USB;
 	dj_hiddev->vendor = le16_to_cpu(usbdev->descriptor.idVendor);
-	dj_hiddev->product = le16_to_cpu(usbdev->descriptor.idProduct);
+	dj_hiddev->product =
+		(dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_MSB]
+									<< 8) |
+		dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_LSB];
 	snprintf(dj_hiddev->name, sizeof(dj_hiddev->name),
-		"Logitech Unifying Device. Wireless PID:%02x%02x",
-		dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_MSB],
-		dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_LSB]);
+		"Logitech Unifying Device. Wireless PID:%04x",
+		dj_hiddev->product);
+
+	dj_hiddev->group = HID_GROUP_LOGITECH_DJ_DEVICE;
 
 	usb_make_path(usbdev, dj_hiddev->phys, sizeof(dj_hiddev->phys));
 	snprintf(tmpstr, sizeof(tmpstr), ":%d", dj_report->device_index);
@@ -714,9 +718,6 @@ static int logi_dj_probe(struct hid_device *hdev,
 	struct dj_receiver_dev *djrcv_dev;
 	int retval;
 
-	if (is_dj_device((struct dj_device *)hdev->driver_data))
-		return -ENODEV;
-
 	dbg_hid("%s called for ifnum %d\n", __func__,
 		intf->cur_altsetting->desc.bInterfaceNumber);
 
@@ -869,22 +870,6 @@ static void logi_dj_remove(struct hid_device *hdev)
 	hid_set_drvdata(hdev, NULL);
 }
 
-static int logi_djdevice_probe(struct hid_device *hdev,
-			 const struct hid_device_id *id)
-{
-	int ret;
-	struct dj_device *dj_dev = hdev->driver_data;
-
-	if (!is_dj_device(dj_dev))
-		return -ENODEV;
-
-	ret = hid_parse(hdev);
-	if (!ret)
-		ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
-
-	return ret;
-}
-
 static const struct hid_device_id logi_dj_receivers[] = {
 	{HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
 		USB_DEVICE_ID_LOGITECH_UNIFYING_RECEIVER)},
@@ -908,17 +893,14 @@ static struct hid_driver logi_djreceiver_driver = {
 
 
 static const struct hid_device_id logi_dj_devices[] = {
-	{HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
-		USB_DEVICE_ID_LOGITECH_UNIFYING_RECEIVER)},
-	{HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
-		USB_DEVICE_ID_LOGITECH_UNIFYING_RECEIVER_2)},
+	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
+		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
 	{}
 };
 
 static struct hid_driver logi_djdevice_driver = {
 	.name = "logitech-djdevice",
 	.id_table = logi_dj_devices,
-	.probe = logi_djdevice_probe,
 };
 
 
diff --git a/drivers/hid/hid-logitech-dj.h b/drivers/hid/hid-logitech-dj.h
index daeb0aa..b1208c9 100644
--- a/drivers/hid/hid-logitech-dj.h
+++ b/drivers/hid/hid-logitech-dj.h
@@ -112,14 +112,4 @@ struct dj_device {
 	u8 device_index;
 };
 
-/**
- * is_dj_device - know if the given dj_device is not the receiver.
- * @dj_dev: the dj device to test
- *
- * This macro tests if a struct dj_device pointer is a device created
- * by the bus enumarator.
- */
-#define is_dj_device(dj_dev) \
-	(&(dj_dev)->dj_receiver_dev->hdev->dev == (dj_dev)->hdev->dev.parent)
-
 #endif
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 0da2711..a50a6dd 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -312,6 +312,7 @@ struct hid_item {
  */
 #define HID_GROUP_RMI				0x0100
 #define HID_GROUP_WACOM				0x0101
+#define HID_GROUP_LOGITECH_DJ_DEVICE		0x0102
 
 /*
  * This is the global environment of the parser. This information is
-- 
2.1.0


^ permalink raw reply related

* Re: [PATCH 0/5] HID: wacom: introduce generic HID handling
From: Benjamin Tissoires @ 2014-09-30 17:27 UTC (permalink / raw)
  To: Jason Gerecke; +Cc: Jiri Kosina, Ping Cheng, Linux Input, linux-kernel
In-Reply-To: <CANRwn3Sng5nxc7YZEUYQVgFhidSMQCBdDsiWebs_zFZ7s=gsiQ@mail.gmail.com>

On Sep 26 2014 or thereabouts, Jason Gerecke wrote:
> On Fri, Sep 26, 2014 at 4:03 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> > On Tue, 23 Sep 2014, Benjamin Tissoires wrote:
> >
> >> Hi guys,
> >>
> >> So, this patch series aims at supporting natively any future HID compliant wacom
> >> tablet. Those found on the various laptops (ISDv4/5) already are HID compliant
> >> and they should work in the future without any modification of the kernel.
> >>
> >> Few things to note here:
> >> - I used the PID 0x00E6 found on the Lenovo X230 for the tests
> >> - I did not removed its entry in the list because there is a slightly different
> >>   behavior while using this patch set: when more than two fingers are on the
> >>   screen, the tablet stops sending finger events, while with the wacom
> >>   proprietary bits, it continues to send them. Besides that, the events emitted
> >>   before and after the patch series are the same (at least on the E6)
> >> - I can not rely on hid-input directly because wacom wants to be in control of
> >>   the input devices it creates. This might be solved in the future, but for now
> >>   it is just easier to rewrite the few mapping/events handling than trying to
> >>   fit in the hid-input model.
> >> - there will still be more specific use cases to handle later (see the joke of
> >>   the MS surface 3 pen[1] for instance), but this should give a roughly working
> >>   pen/touch support for those future devices.
> >>
> >> Jason, I would be very glad if you could conduct a few tests for this on the
> >> ISDv4/5 sensors you have.
> >
> > I am waiting with this one for testing results from Jason, but if you guys
> > want this to go in the next merge window, I'd like to ask you not to
> > postpone the testing too much.
> >
> > Thanks.
> >
> 
> I'm in the process of capturing traces from hid-record for multiple
> tablet PCs both pre- and post-patch to locate any obvious issues. My
> initial results are promising on the pen side (the only things I've
> noticed so far is the 2nd barrel switch not being supported -- as
> might be suspected) but less so on the touch side (single-finger
> devices seem to work fine, but the new code doesn't seem to handle any
> of the multitouch devices I've tried).

Do you consider not having the second barrel switch a blocker or can we
ship it this and send a fix later?

As for the touch, I tested/developed it on the Lenovo X230. It presents
a hid-multitouch compatible touch interface, when the feature input_mode
is sent with the value '2'. By re-reading it, it seems to me that you
are trying to replay the touch traces you got before the patch, but this
will not work because the touch protocol is not the same. ISDv4 used to
rely on the proprietary touch protocol which is hard to describe in HID.

I would say that as long as there is no regression (actually there can
not be for old devices :-P), we should still carry this series for 3.18.
We might need to adjust the bits here and there, but if we can have a
*basic* support of the tablet out of the box, this is still much better
than silently ignoring it.

> 
> Expect more news next week :)

So?????

> 
> 
> PS: I'm pretty sure ISDv5 shouldn't be relied on for HID data :| The
> descriptor for the Cintiq Companion Hybrid, at least, is like our
> branded sensors with useful data only in the touchscreen descriptor
> (the pen descriptor is a bunch of opaque vendor-defined reports). Not
> sure if the Cintiq Companion is the same though since it runs
> Windows...

Ouch. For those, we will need to either fix the report descriptors or
handle them in the same way you always used to do: manually.

Cheers,
Benjamin

> >>
> >> [1] http://who-t.blogspot.com/2014/09/stylus-behaviour-on-microsoft-surface-3.html
> >>
> >> Benjamin Tissoires (5):
> >>   HID: wacom: rename failN with some meaningful information
> >>   HID: wacom: split out input allocation and registration
> >>   HID: wacom: move allocation of inputs earlier
> >>   HID: wacom: implement generic HID handling for pen generic devices
> >>   HID: wacom: implement the finger part of the HID generic handling
> >>
> >>  drivers/hid/hid-core.c  |   3 +
> >>  drivers/hid/wacom.h     |   6 +
> >>  drivers/hid/wacom_sys.c | 184 +++++++++++++++++++++--------
> >>  drivers/hid/wacom_wac.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  drivers/hid/wacom_wac.h |  17 +++
> >>  include/linux/hid.h     |   2 +
> >>  6 files changed, 462 insertions(+), 49 deletions(-)
> >>
> >> --
> >> 2.1.0
> >>
> >
> > --
> > Jiri Kosina
> > SUSE Labs

^ permalink raw reply

* [PATCH] HID: usbhid: prevent unwanted events to be sent when re-opening the device
From: Benjamin Tissoires @ 2014-09-30 18:28 UTC (permalink / raw)
  To: Jiri Kosina, Hans de Goede; +Cc: Bastien Nocera, linux-kernel, linux-input

When events occurs while no one is listening to the node (hid->open == 0
and usb_kill_urb() called) some events are still stacked somewhere in
the USB (kernel or device?) stack. When the node gets reopened, these
events are drained, and this results in spurious touch down/up, or mouse
button clicks.

The problem was spotted with touchscreens in fdo bug #81781 [1], but it
actually occurs with any mouse using hid-generic or touchscreen.

A way to reproduce it is to call:

$ xinput disable 9 ; sleep 5 ; xinput enable 9

With 9 being the device ID for the touchscreen/mouse. During the "sleep",
produce some touch events or click events. When "xinput enable" is called,
at least one click is generated.

This patch tries to fix this by draining the queue for 50 msec and
during this time frame, not forwarding these old events to the hid layer.

Hans completed the explanation:
"""
Devices like mice (basically any hid device) will have a fifo
on the device side, when we stop submitting urbs to get hid reports from
it, that fifo will fill up, and when we resume we will get whatever
is there in that fifo.
"""

[1] https://bugs.freedesktop.org/show_bug.cgi?id=81781

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

OK, I know this is uggly, but I could not find a better way :(

Cheers,
Benjamin

 drivers/hid/usbhid/hid-core.c | 36 ++++++++++++++++++++++++------------
 drivers/hid/usbhid/usbhid.h   |  1 +
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index ca6849a..04e34b9 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -278,18 +278,20 @@ static void hid_irq_in(struct urb *urb)
 		usbhid->retry_delay = 0;
 		if ((hid->quirks & HID_QUIRK_ALWAYS_POLL) && !hid->open)
 			break;
-		hid_input_report(urb->context, HID_INPUT_REPORT,
-				 urb->transfer_buffer,
-				 urb->actual_length, 1);
-		/*
-		 * autosuspend refused while keys are pressed
-		 * because most keyboards don't wake up when
-		 * a key is released
-		 */
-		if (hid_check_keys_pressed(hid))
-			set_bit(HID_KEYS_PRESSED, &usbhid->iofl);
-		else
-			clear_bit(HID_KEYS_PRESSED, &usbhid->iofl);
+		if (!test_bit(HID_RESUME_RUNNING, &usbhid->iofl)) {
+			hid_input_report(urb->context, HID_INPUT_REPORT,
+					 urb->transfer_buffer,
+					 urb->actual_length, 1);
+			/*
+			 * autosuspend refused while keys are pressed
+			 * because most keyboards don't wake up when
+			 * a key is released
+			 */
+			if (hid_check_keys_pressed(hid))
+				set_bit(HID_KEYS_PRESSED, &usbhid->iofl);
+			else
+				clear_bit(HID_KEYS_PRESSED, &usbhid->iofl);
+		}
 		break;
 	case -EPIPE:		/* stall */
 		usbhid_mark_busy(usbhid);
@@ -688,6 +690,7 @@ int usbhid_open(struct hid_device *hid)
 			goto done;
 		}
 		usbhid->intf->needs_remote_wakeup = 1;
+		set_bit(HID_RESUME_RUNNING, &usbhid->iofl);
 		res = hid_start_in(hid);
 		if (res) {
 			if (res != -ENOSPC) {
@@ -701,6 +704,15 @@ int usbhid_open(struct hid_device *hid)
 			}
 		}
 		usb_autopm_put_interface(usbhid->intf);
+
+		/*
+		 * In case events are generated while nobody was listening,
+		 * some are released when the device is re-opened.
+		 * Wait 50 msec for the queue to empty before allowing events
+		 * to go through hid.
+		 */
+		msleep(50);
+		clear_bit(HID_RESUME_RUNNING, &usbhid->iofl);
 	}
 done:
 	mutex_unlock(&hid_open_mut);
diff --git a/drivers/hid/usbhid/usbhid.h b/drivers/hid/usbhid/usbhid.h
index f633c24..807922b 100644
--- a/drivers/hid/usbhid/usbhid.h
+++ b/drivers/hid/usbhid/usbhid.h
@@ -52,6 +52,7 @@ struct usb_interface *usbhid_find_interface(int minor);
 #define HID_STARTED		8
 #define HID_KEYS_PRESSED	10
 #define HID_NO_BANDWIDTH	11
+#define HID_RESUME_RUNNING	12
 
 /*
  * USB-specific HID struct, to be pointed to
-- 
2.1.0


^ permalink raw reply related

* [PATCH 1/3] input: tsc2007: Add pre-calibration, flipping and rotation
From: Marek Belisko @ 2014-09-30 20:17 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	bcousson, tony, linux, dmitry.torokhov
  Cc: devicetree, hns, linux-kernel, linux-input, Marek Belisko,
	linux-omap, linux-arm-kernel
In-Reply-To: <1412108254-19234-1-git-send-email-marek@goldelico.com>

This patch adds new parameters that allow to address typical hardware
design differences: touch screens may be wired or oriented differently
(portrait or landscape). And usually the active area of the touch is a
little larger than the active area of the LCD. This results in the touch
coordinates that have some significant deviation from LCD coordinates.
Usually this is addressed in user space by a calibration tool (e.g. tslib
or xinput-calibrator) but some systems don't have these tools or require
that the screen is already roughly calibrated (e.g. Replicant) to operate
the device until a better calibration can be done. And, some systems
react very strangely if the touch event stream reports coordinates
outside of the active area.

This makes it necessry to be able to configure:
1. swapping x and y wires (coordinate values)
2. flipping of x (left - right) or y (top - bottom) or even both
3. define an active area so that an uncalibrated screen already
roughly matches the LCD to be useful.
4. clip reported coordinates to the active area.

If none of the new parameters is defined (in DT) or set in a board file,
the driver behaves the same as without this patch.

Author (1&2): H. Nikolaus Schaller <hns@goldelico.com>
Author (3&4): Paul Kocialkowski <contact@paulk.fr>

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/input/touchscreen/tsc2007.c | 89 +++++++++++++++++++++++++++++++++++--
 include/linux/i2c/tsc2007.h         |  6 +++
 2 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
index 1bf9906..cc0cc3c 100644
--- a/drivers/input/touchscreen/tsc2007.c
+++ b/drivers/input/touchscreen/tsc2007.c
@@ -74,6 +74,12 @@ struct tsc2007 {
 
 	u16			model;
 	u16			x_plate_ohms;
+	bool			swap_xy;	/* swap x and y axis */
+	u16			min_x;
+	u16			min_y;
+	u16			min_rt;
+	u16			max_x;
+	u16			max_y;
 	u16			max_rt;
 	unsigned long		poll_period;
 	int			fuzzx;
@@ -193,11 +199,50 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 			break;
 		}
 
-		if (rt <= ts->max_rt) {
+		if (rt <= max(ts->min_rt, ts->max_rt)) {
 			dev_dbg(&ts->client->dev,
 				"DOWN point(%4d,%4d), pressure (%4u)\n",
 				tc.x, tc.y, rt);
 
+			if (ts->swap_xy) {
+				/* swap before applying the range limits */
+				u16 h = tc.x;
+
+				tc.x = tc.y;
+				tc.y = h;
+			}
+
+			/* flip and/or clip X */
+			if (ts->max_x < ts->min_x)
+				tc.x = (ts->min_x - tc.x) + ts->max_x;
+
+			if (tc.x > max(ts->min_x, ts->max_x))
+				tc.x = max(ts->min_x, ts->max_x);
+			else if (tc.x < min(ts->min_x, ts->max_x))
+				tc.x = min(ts->min_x, ts->max_x);
+
+			/* flip and/or clip Y */
+			if (ts->max_y < ts->min_y)
+				tc.y = (ts->min_y - tc.y) + ts->max_y;
+
+			if (tc.y > max(ts->min_y, ts->max_y))
+				tc.y = max(ts->min_y, ts->max_y);
+			else if (tc.y < min(ts->min_y, ts->max_y))
+				tc.y = min(ts->min_y, ts->max_y);
+
+			/* clip Z */
+			if (ts->max_rt < ts->min_rt)
+				rt = (ts->min_rt - rt) + ts->max_rt;
+
+			if (rt > max(ts->min_rt, ts->max_rt))
+				rt = max(ts->min_rt, ts->max_rt);
+			else if (rt < min(ts->min_rt, ts->max_rt))
+				rt = min(ts->min_rt, ts->max_rt);
+
+			dev_dbg(&ts->client->dev,
+					"shaped point(%4d,%4d), pressure (%4u)\n",
+					tc.x, tc.y, rt);
+
 			input_report_key(input, BTN_TOUCH, 1);
 			input_report_abs(input, ABS_X, tc.x);
 			input_report_abs(input, ABS_Y, tc.y);
@@ -299,6 +344,24 @@ static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts)
 		return -EINVAL;
 	}
 
+	ts->swap_xy = of_property_read_bool(np, "ti,swap-xy");
+
+	if (!of_property_read_u32(np, "ti,min-x", &val32))
+		ts->min_x = val32;
+	if (!of_property_read_u32(np, "ti,max-x", &val32))
+		ts->max_x = val32;
+	else
+		ts->max_x = MAX_12BIT;
+
+	if (!of_property_read_u32(np, "ti,min-y", &val32))
+		ts->min_y = val32;
+	if (!of_property_read_u32(np, "ti,max-y", &val32))
+		ts->max_y = val32;
+	else
+		ts->max_y = MAX_12BIT;
+
+	if (!of_property_read_u32(np, "ti,min-rt", &val32))
+		ts->min_rt = val32;
 	if (!of_property_read_u32(np, "ti,max-rt", &val32))
 		ts->max_rt = val32;
 	else
@@ -325,6 +388,16 @@ static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts)
 		return -EINVAL;
 	}
 
+	dev_dbg(&client->dev,
+			"min/max_x (%4d,%4d)\n",
+			ts->min_x, ts->max_x);
+	dev_dbg(&client->dev,
+			"min/max_y (%4d,%4d)\n",
+			ts->min_y, ts->max_y);
+	dev_dbg(&client->dev,
+			"min/max_rt (%4d,%4d)\n",
+			ts->min_rt, ts->max_rt);
+
 	ts->gpio = of_get_gpio(np, 0);
 	if (gpio_is_valid(ts->gpio))
 		ts->get_pendown_state = tsc2007_get_pendown_state_gpio;
@@ -349,6 +422,12 @@ static int tsc2007_probe_pdev(struct i2c_client *client, struct tsc2007 *ts,
 {
 	ts->model             = pdata->model;
 	ts->x_plate_ohms      = pdata->x_plate_ohms;
+	ts->swap_xy           = pdata->swap_xy;
+	ts->min_x             = pdata->min_x ? : 0;
+	ts->min_y             = pdata->min_y ? : 0;
+	ts->min_rt            = pdata->min_rt ? : 0;
+	ts->max_x             = pdata->max_x ? : MAX_12BIT;
+	ts->max_y             = pdata->max_y ? : MAX_12BIT;
 	ts->max_rt            = pdata->max_rt ? : MAX_12BIT;
 	ts->poll_period       = pdata->poll_period ? : 1;
 	ts->get_pendown_state = pdata->get_pendown_state;
@@ -422,9 +501,11 @@ static int tsc2007_probe(struct i2c_client *client,
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
 	input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
 
-	input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, ts->fuzzx, 0);
-	input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, ts->fuzzy, 0);
-	input_set_abs_params(input_dev, ABS_PRESSURE, 0, MAX_12BIT,
+	input_set_abs_params(input_dev, ABS_X, min(ts->min_x, ts->max_x),
+			     max(ts->min_x, ts->max_x), ts->fuzzx, 0);
+	input_set_abs_params(input_dev, ABS_Y, min(ts->min_y, ts->max_y),
+			     max(ts->min_y, ts->max_y), ts->fuzzy, 0);
+	input_set_abs_params(input_dev, ABS_PRESSURE, ts->min_rt, ts->max_rt,
 			     ts->fuzzz, 0);
 
 	if (pdata) {
diff --git a/include/linux/i2c/tsc2007.h b/include/linux/i2c/tsc2007.h
index 4f35b6a..5ce7b79 100644
--- a/include/linux/i2c/tsc2007.h
+++ b/include/linux/i2c/tsc2007.h
@@ -6,6 +6,12 @@
 struct tsc2007_platform_data {
 	u16	model;				/* 2007. */
 	u16	x_plate_ohms;	/* must be non-zero value */
+	bool	swap_xy;	/* swap x and y axis */
+	u16	min_x;	/* min and max values to reported to user space */
+	u16	min_y;
+	u16	min_rt;
+	u16	max_x;
+	u16	max_y;
 	u16	max_rt; /* max. resistance above which samples are ignored */
 	unsigned long poll_period; /* time (in ms) between samples */
 	int	fuzzx; /* fuzz factor for X, Y and pressure axes */
-- 
1.9.3

^ permalink raw reply related

* [PATCH 2/3] Documentation: dt: input: tsc2007: Document new parameters
From: Marek Belisko @ 2014-09-30 20:17 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	bcousson, tony, linux, dmitry.torokhov
  Cc: hns, devicetree, linux-kernel, linux-omap, linux-arm-kernel,
	linux-input, Marek Belisko
In-Reply-To: <1412108254-19234-1-git-send-email-marek@goldelico.com>

Signed-off-by: Marek Belisko <marek@goldelico.com>
---
 .../devicetree/bindings/input/touchscreen/tsc2007.txt   | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
index ec365e1..46b086c 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
@@ -13,14 +13,25 @@ Optional properties:
   (see interrupt binding[0]).
 - interrupts: (gpio) interrupt to which the chip is connected
   (see interrupt binding[0]).
-- ti,max-rt: maximum pressure.
+- ti,swap-xy: if present, swap x and y values. Rotation left/right is
+  achieved by combination with flipping of x or y.
+- ti,min-x: minimum x (default 0). Use this for a coarse calibration of the
+  touch if there is no user space option (e.g. Android, X11).
+- ti,max-x: maximum x (default 4095). If max-x is smaller than min-x the
+  axis is swapped.
+- ti,min-y: minimum y (default 0).
+- ti,max-y: maximum y (default 4095).
+- ti,min-rt: minimum pressure (default 0).
+- ti,max-rt: maximum pressure (default 4095). Depending on your needs, you
+  can also invert the pressure value since the raw rt value  reports the
+  resistance and not the "pressure" (i.e. becomes lower for higher pressure).
 - ti,fuzzx: specifies the absolute input fuzz x value.
   If set, it will permit noise in the data up to +- the value given to the fuzz
-  parameter, that is used to filter noise from the event stream.
+  parameter, that is used to filter noise from the event stream (default 0).
 - ti,fuzzy: specifies the absolute input fuzz y value.
 - ti,fuzzz: specifies the absolute input fuzz z value.
 - ti,poll-period: how much time to wait (in milliseconds) before reading again the
-  values from the tsc2007.
+  values from the tsc2007 (default 1).
 
 [0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
 [1]: Documentation/devicetree/bindings/gpio/gpio.txt
-- 
1.9.3

^ permalink raw reply related

* [PATCH 3/3] arm: dts: omap3-gta04: Extend touchscreen configs
From: Marek Belisko @ 2014-09-30 20:17 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	bcousson, tony, linux, dmitry.torokhov
  Cc: hns, devicetree, linux-kernel, linux-omap, linux-arm-kernel,
	linux-input, Marek Belisko
In-Reply-To: <1412108254-19234-1-git-send-email-marek@goldelico.com>

Adding min/max values for various touschscreen properties.

Signed-off-by: Marek Belisko <marek@goldelico.com>
---
 arch/arm/boot/dts/omap3-gta04.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi
index fd34f91..2dafac3b 100644
--- a/arch/arm/boot/dts/omap3-gta04.dtsi
+++ b/arch/arm/boot/dts/omap3-gta04.dtsi
@@ -284,6 +284,12 @@
 		interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
 		gpios = <&gpio6 0 GPIO_ACTIVE_LOW>;
 		ti,x-plate-ohms = <600>;
+		ti,min-x = <0x100>;
+		ti,max-x = <0xf00>;
+		ti,min-y = <0xf00>;
+		ti,max-y = <0x100>;
+		ti,min-rt = <0xfff>;
+		ti,max-rt = <0>;
 	};
 };
 
-- 
1.9.3

^ permalink raw reply related

* [PATCH 0/3] input: tsc2007: Extend for pre-calibration, flipping and rotation
From: Marek Belisko @ 2014-09-30 20:17 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	bcousson, tony, linux, dmitry.torokhov
  Cc: hns, devicetree, linux-kernel, linux-omap, linux-arm-kernel,
	linux-input, Marek Belisko

Following series add support to tsc2007 touchscreen driver for pre-calibration,
flipping and rotation. Added bindings are documented and used in gta04 device tree.

Marek Belisko (3):
  input: tsc2007: Add pre-calibration, flipping and rotation
  Documentation: dt: input: tsc2007: Document new parameters
  arm: dts: omap3-gta04: Extend touchscreen configs

 .../bindings/input/touchscreen/tsc2007.txt         | 17 ++++-
 arch/arm/boot/dts/omap3-gta04.dtsi                 |  6 ++
 drivers/input/touchscreen/tsc2007.c                | 89 +++++++++++++++++++++-
 include/linux/i2c/tsc2007.h                        |  6 ++
 4 files changed, 111 insertions(+), 7 deletions(-)

-- 
1.9.3


^ permalink raw reply

* Re: [PATCH 0/5] HID: wacom: introduce generic HID handling
From: Jason Gerecke @ 2014-09-30 23:46 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Jiri Kosina, Ping Cheng, Linux Input, linux-kernel
In-Reply-To: <20140930172718.GA30615@mail.corp.redhat.com>

On Tue, Sep 30, 2014 at 10:27 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> On Sep 26 2014 or thereabouts, Jason Gerecke wrote:
>> On Fri, Sep 26, 2014 at 4:03 AM, Jiri Kosina <jkosina@suse.cz> wrote:
>> > On Tue, 23 Sep 2014, Benjamin Tissoires wrote:
>> >
>> >> Hi guys,
>> >>
>> >> So, this patch series aims at supporting natively any future HID compliant wacom
>> >> tablet. Those found on the various laptops (ISDv4/5) already are HID compliant
>> >> and they should work in the future without any modification of the kernel.
>> >>
>> >> Few things to note here:
>> >> - I used the PID 0x00E6 found on the Lenovo X230 for the tests
>> >> - I did not removed its entry in the list because there is a slightly different
>> >>   behavior while using this patch set: when more than two fingers are on the
>> >>   screen, the tablet stops sending finger events, while with the wacom
>> >>   proprietary bits, it continues to send them. Besides that, the events emitted
>> >>   before and after the patch series are the same (at least on the E6)
>> >> - I can not rely on hid-input directly because wacom wants to be in control of
>> >>   the input devices it creates. This might be solved in the future, but for now
>> >>   it is just easier to rewrite the few mapping/events handling than trying to
>> >>   fit in the hid-input model.
>> >> - there will still be more specific use cases to handle later (see the joke of
>> >>   the MS surface 3 pen[1] for instance), but this should give a roughly working
>> >>   pen/touch support for those future devices.
>> >>
>> >> Jason, I would be very glad if you could conduct a few tests for this on the
>> >> ISDv4/5 sensors you have.
>> >
>> > I am waiting with this one for testing results from Jason, but if you guys
>> > want this to go in the next merge window, I'd like to ask you not to
>> > postpone the testing too much.
>> >
>> > Thanks.
>> >
>>
>> I'm in the process of capturing traces from hid-record for multiple
>> tablet PCs both pre- and post-patch to locate any obvious issues. My
>> initial results are promising on the pen side (the only things I've
>> noticed so far is the 2nd barrel switch not being supported -- as
>> might be suspected) but less so on the touch side (single-finger
>> devices seem to work fine, but the new code doesn't seem to handle any
>> of the multitouch devices I've tried).
>
> Do you consider not having the second barrel switch a blocker or can we
> ship it this and send a fix later?
>
> As for the touch, I tested/developed it on the Lenovo X230. It presents
> a hid-multitouch compatible touch interface, when the feature input_mode
> is sent with the value '2'. By re-reading it, it seems to me that you
> are trying to replay the touch traces you got before the patch, but this
> will not work because the touch protocol is not the same. ISDv4 used to
> rely on the proprietary touch protocol which is hard to describe in HID.
>
> I would say that as long as there is no regression (actually there can
> not be for old devices :-P), we should still carry this series for 3.18.
> We might need to adjust the bits here and there, but if we can have a
> *basic* support of the tablet out of the box, this is still much better
> than silently ignoring it.
>
I'm in agreement. The lack of second-switch support was expected and
somewhat trivial at this moment. We can work on finessing it as we go.

Regarding the touch input, I haven't had enough time to dive in to
figuring out what is wrong, but traces aren't the issue (I've not been
able to use traces for multitouch testing because the driver can't
query features->touch_max ;)). Working with actual hardware, I don't
get _any_ events out of the driver when using the touchscreen. I
checked that the device is sending the non-vendor-defined reports,
which does seem to be the case. The descriptor for the reports also
seem to be reasonable, appearing to follow Microsoft's
recommendations...

>>
>> Expect more news next week :)
>
> So?????
>
I'm not sure if you want to fix touch first or just get these upstream
and fix during RC (as you said, it won't cause any regressions...) but
I'll give the following:

Acked-by: Jason Gerecke <killertofu@gmail.com>

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....

>>
>>
>> PS: I'm pretty sure ISDv5 shouldn't be relied on for HID data :| The
>> descriptor for the Cintiq Companion Hybrid, at least, is like our
>> branded sensors with useful data only in the touchscreen descriptor
>> (the pen descriptor is a bunch of opaque vendor-defined reports). Not
>> sure if the Cintiq Companion is the same though since it runs
>> Windows...
>
> Ouch. For those, we will need to either fix the report descriptors or
> handle them in the same way you always used to do: manually.
>
> Cheers,
> Benjamin
>
>> >>
>> >> [1] http://who-t.blogspot.com/2014/09/stylus-behaviour-on-microsoft-surface-3.html
>> >>
>> >> Benjamin Tissoires (5):
>> >>   HID: wacom: rename failN with some meaningful information
>> >>   HID: wacom: split out input allocation and registration
>> >>   HID: wacom: move allocation of inputs earlier
>> >>   HID: wacom: implement generic HID handling for pen generic devices
>> >>   HID: wacom: implement the finger part of the HID generic handling
>> >>
>> >>  drivers/hid/hid-core.c  |   3 +
>> >>  drivers/hid/wacom.h     |   6 +
>> >>  drivers/hid/wacom_sys.c | 184 +++++++++++++++++++++--------
>> >>  drivers/hid/wacom_wac.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  drivers/hid/wacom_wac.h |  17 +++
>> >>  include/linux/hid.h     |   2 +
>> >>  6 files changed, 462 insertions(+), 49 deletions(-)
>> >>
>> >> --
>> >> 2.1.0
>> >>
>> >
>> > --
>> > Jiri Kosina
>> > SUSE Labs

^ permalink raw reply

* Re: [PATCH 0/5] HID: wacom: introduce generic HID handling
From: Jiri Kosina @ 2014-10-01  7:14 UTC (permalink / raw)
  To: Jason Gerecke; +Cc: Benjamin Tissoires, Ping Cheng, Linux Input, linux-kernel
In-Reply-To: <CANRwn3Sf=288SNTTbJT11gJFUTZYGetHypaECKxy8gBzyVpaVg@mail.gmail.com>

Applied the series to for-3.18/wacom. Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: cp2112: remove use of gpiochip_remove() retval
From: Jiri Kosina @ 2014-10-01  7:18 UTC (permalink / raw)
  To: Pramod Gurav
  Cc: linux-kernel, Linus Walleij, Alexandre Courbot, linux-input,
	linux-gpio
In-Reply-To: <1412087049-4368-1-git-send-email-pramod.gurav@smartplayin.com>

On Tue, 30 Sep 2014, Pramod Gurav wrote:

> Get rid of using return value from gpiochip_remove() as it returns
> void.
> 
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: linux-input@vger.kernel.org 
> Cc: linux-gpio@vger.kernel.org
> 
> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>

I guess this depends on some other patch that hasn't gone to Linus yet, 
right? As Linus' tree currently has this prototype:

	int gpiochip_remove(struct gpio_chip *chip)

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] usbhid: add another mouse that needs QUIRK_ALWAYS_POLL
From: Jiri Kosina @ 2014-10-01  7:23 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: linux-input, johan
In-Reply-To: <1412074496-4539-1-git-send-email-oneukum@suse.de>

On Tue, 30 Sep 2014, Oliver Neukum wrote:

> There is a second mouse sharing the same vendor strings
> but different IDs.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.de>
> ---
>  drivers/hid/hid-ids.h           | 1 +
>  drivers/hid/usbhid/hid-quirks.c | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index b303a62..5a7072d 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -733,6 +733,7 @@
>  #define USB_DEVICE_ID_PI_ENGINEERING_VEC_USB_FOOTPEDAL	0xff
>  
>  #define USB_VENDOR_ID_PIXART				0x093a
> +#define USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE_ID2	0x0137
>  #define USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE		0x2510
>  #define USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN	0x8001
>  #define USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN1	0x8002
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 79f94ae..ce27c62 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -80,6 +80,8 @@ static const struct hid_blacklist {
>  	{ USB_VENDOR_ID_PENMOUNT, USB_DEVICE_ID_PENMOUNT_1610, HID_QUIRK_NOGET },
>  	{ USB_VENDOR_ID_PENMOUNT, USB_DEVICE_ID_PENMOUNT_1640, HID_QUIRK_NOGET },
>  	{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE, HID_QUIRK_ALWAYS_POLL },
> +	/* OEM version same strings but different IDs */
> +	{ USB_VENDOR_ID_KYE, USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE_ID2, HID_QUIRK_ALWAYS_POLL },

I don't like the added comment, it makes the list look ugly (as no other 
device has such a comment). The details are in the git changelog anyway.

Are you fine with me removing the comment before commiting the patch?

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: cp2112: remove use of gpiochip_remove() retval
From: Pramod Gurav @ 2014-10-01  7:26 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: linux-kernel, Linus Walleij, Alexandre Courbot, linux-input,
	linux-gpio
In-Reply-To: <alpine.LNX.2.00.1410010917000.26311@pobox.suse.cz>

On Wednesday 01 October 2014 12:48 PM, Jiri Kosina wrote:
>> > Get rid of using return value from gpiochip_remove() as it returns
>> > void.
>> > 
>> > Cc: Linus Walleij <linus.walleij@linaro.org>
>> > Cc: Alexandre Courbot <gnurou@gmail.com>
>> > Cc: Jiri Kosina <jkosina@suse.cz>
>> > Cc: linux-input@vger.kernel.org 
>> > Cc: linux-gpio@vger.kernel.org
>> > 
>> > Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
> I guess this depends on some other patch that hasn't gone to Linus yet, 
> right? As Linus' tree currently has this prototype:
> 
> 	int gpiochip_remove(struct gpio_chip *chip)

Yes. It is yet to make it to linus's tree. But the change is there in
linux-next master branch.


Thanks
Pramod

^ permalink raw reply

* Re: [PATCH] HID: cp2112: remove use of gpiochip_remove() retval
From: Jiri Kosina @ 2014-10-01  8:01 UTC (permalink / raw)
  To: Pramod Gurav
  Cc: linux-kernel, Linus Walleij, Alexandre Courbot, linux-input,
	linux-gpio
In-Reply-To: <542BACB3.3010203@smartplayin.com>

On Wed, 1 Oct 2014, Pramod Gurav wrote:

> >> > Cc: Linus Walleij <linus.walleij@linaro.org>
> >> > Cc: Alexandre Courbot <gnurou@gmail.com>
> >> > Cc: Jiri Kosina <jkosina@suse.cz>
> >> > Cc: linux-input@vger.kernel.org 
> >> > Cc: linux-gpio@vger.kernel.org
> >> > 
> >> > Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
> > I guess this depends on some other patch that hasn't gone to Linus yet, 
> > right? As Linus' tree currently has this prototype:
> > 
> > 	int gpiochip_remove(struct gpio_chip *chip)
> 
> Yes. It is yet to make it to linus's tree. But the change is there in
> linux-next master branch.

Alright. It probably makes most sense to take this patch together with the 
prototype change in one pile (i.e. not through my tree).

For that purpose

	Acked-by: Jiri Kosina <jkosina@suse.cz>

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] usbhid: add another mouse that needs QUIRK_ALWAYS_POLL
From: Oliver Neukum @ 2014-10-01  8:14 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, johan
In-Reply-To: <alpine.LNX.2.00.1410010921570.26311@pobox.suse.cz>

On Wed, 2014-10-01 at 09:23 +0200, Jiri Kosina wrote:
> On Tue, 30 Sep 2014, Oliver Neukum wrote:
> 
> > There is a second mouse sharing the same vendor strings
> > but different IDs.
> > 
> > Signed-off-by: Oliver Neukum <oneukum@suse.de>
> > ---
> >  drivers/hid/hid-ids.h           | 1 +
> >  drivers/hid/usbhid/hid-quirks.c | 2 ++
> >  2 files changed, 3 insertions(+)
> > 
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index b303a62..5a7072d 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -733,6 +733,7 @@
> >  #define USB_DEVICE_ID_PI_ENGINEERING_VEC_USB_FOOTPEDAL	0xff
> >  
> >  #define USB_VENDOR_ID_PIXART				0x093a
> > +#define USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE_ID2	0x0137
> >  #define USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE		0x2510
> >  #define USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN	0x8001
> >  #define USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN1	0x8002
> > diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> > index 79f94ae..ce27c62 100644
> > --- a/drivers/hid/usbhid/hid-quirks.c
> > +++ b/drivers/hid/usbhid/hid-quirks.c
> > @@ -80,6 +80,8 @@ static const struct hid_blacklist {
> >  	{ USB_VENDOR_ID_PENMOUNT, USB_DEVICE_ID_PENMOUNT_1610, HID_QUIRK_NOGET },
> >  	{ USB_VENDOR_ID_PENMOUNT, USB_DEVICE_ID_PENMOUNT_1640, HID_QUIRK_NOGET },
> >  	{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE, HID_QUIRK_ALWAYS_POLL },
> > +	/* OEM version same strings but different IDs */
> > +	{ USB_VENDOR_ID_KYE, USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE_ID2, HID_QUIRK_ALWAYS_POLL },
> 
> I don't like the added comment, it makes the list look ugly (as no other 
> device has such a comment). The details are in the git changelog anyway.
> 
> Are you fine with me removing the comment before commiting the patch?

If you wish to remove it, please do so. However, if people then complain
that I mixed up vendor IDs, will you handle that? ;)

	Regards
		Oliver




^ permalink raw reply

* Re: [PATCH 00/13] HID: add support of Logitech touchpads and special devices
From: Jiri Kosina @ 2014-10-01  8:17 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Nestor Lopez Casado, Andrew de los Reyes, linux-kernel,
	linux-input
In-Reply-To: <1412097515-17241-1-git-send-email-benjamin.tissoires@redhat.com>

On Tue, 30 Sep 2014, Benjamin Tissoires wrote:

> Jiri, This patch series can be split in 2 if you would like:
> - patches 1 to 4 can be scheduled easily for 3.18 IMO
> - patches 5 to 13 might need some more reviews, so either 3.18 or 3.19

Benjamin,

thanks for the summary. I'll try to review 1-4 in time for 3.18, but I 
can't guarantee that.

Do you forsee any issues if the whole lot is queued for 3.19 only?

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: cp2112: remove use of gpiochip_remove() retval
From: Pramod Gurav @ 2014-10-01  8:19 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: linux-kernel, Linus Walleij, Alexandre Courbot, linux-input,
	linux-gpio
In-Reply-To: <alpine.LNX.2.00.1410011000450.26311@pobox.suse.cz>

On Wednesday 01 October 2014 01:31 PM, Jiri Kosina wrote:
> On Wed, 1 Oct 2014, Pramod Gurav wrote:
> 
>>>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>>>> Cc: Alexandre Courbot <gnurou@gmail.com>
>>>>> Cc: Jiri Kosina <jkosina@suse.cz>
>>>>> Cc: linux-input@vger.kernel.org 
>>>>> Cc: linux-gpio@vger.kernel.org
>>>>>
>>>>> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
>>> I guess this depends on some other patch that hasn't gone to Linus yet, 
>>> right? As Linus' tree currently has this prototype:
>>>
>>> 	int gpiochip_remove(struct gpio_chip *chip)
>>
>> Yes. It is yet to make it to linus's tree. But the change is there in
>> linux-next master branch.
> 
> Alright. It probably makes most sense to take this patch together with the 
> prototype change in one pile (i.e. not through my tree).
> 
> For that purpose
> 
> 	Acked-by: Jiri Kosina <jkosina@suse.cz>

Jiri,

Just realized when Linus told me these changes are already in his tree
as well as in mast of linux-tree. Sorry for spaming and late realizing this.

> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox