From de3fb182135224d8285497c2b83c9ca1d8c5a84b Mon Sep 17 00:00:00 2001
From: Andrew de los Reyes <adlr@chromium.org>
Date: Thu, 29 Aug 2013 16:53:15 -0700
Subject: [PATCH] CHROMEOS: HID: Workaround for Logitech Unifying Receiver on
 XHCI systems

This patch addresses the following problem: On some systems (at least
XHCI, it seems), either the request to enumerate connected devices
issued during probe(), or the reply packets, fail. The fallout of this
is that the driver never allocates the downstream devices
(e.g. keyboards and mice).

To fix, we do the following:

- No longer issue a request to enumerate devices during probe. This
  was often failing anyway.

- When we get a message for a nonexisting device (often
  CONNECTION_STATUS set to connected), send a request to enumerate
  devices at that time.

- Before, we'd keep track of whether a device was connected in a
  struct that was allocated when the device was paired. However, with
  the changes above, we now get connected set before the device is
  paired, so we move this status out from the downstream device's
  struct and into the receiver struct.

- When a device is paired, if it was already known to be connected
  previously, we handle it being connected at this time. This means
  allocating the Linux HID input device (something that happens when a
  device is connected, not paired).

With these changes, Logitech devices work on our problematic XHCI
systems.

BUG=chrome-os-partner:22224
TEST=Tested M570 mouse, K400 keyboard, and Logitech T650 touchpad on
Falco. Tested both insertion/removal of receiver and boot while
inserted.

Change-Id: I088a38164fb87ef1328fdc9047cb15f8505630f2
Signed-off-by: Andrew de los Reyes <adlr@chromium.org>
---
 drivers/hid/hid-logitech-dj.c | 84 ++++++++++++++++++++++---------------------
 1 file changed, 43 insertions(+), 41 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 3f9e6a1..b5913a5 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -109,6 +109,7 @@ struct dj_receiver_dev {
 	struct kfifo notif_fifo;
 	spinlock_t lock;
 	bool querying_devices;
+	bool connected[DJ_MAX_PAIRED_DEVICES + DJ_DEVICE_INDEX_MIN];
 };
 
 struct dj_device {
@@ -320,8 +321,8 @@ static void logi_dj_recv_destroy_djhid_device(struct dj_receiver_dev *djrcv_dev,
 	}
 }
 
-static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
-					  struct dj_report *dj_report)
+static int logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
+					 struct dj_report *dj_report)
 {
 	/* Called in delayed work context */
 	struct hid_device *djrcv_hdev = djrcv_dev->hdev;
@@ -339,27 +340,27 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
 	    SPFUNCTION_DEVICE_LIST_EMPTY) {
 		dbg_hid("%s: device list is empty\n", __func__);
 		djrcv_dev->querying_devices = false;
-		return;
+		return -1;
 	}
 
 	if ((dj_report->device_index < DJ_DEVICE_INDEX_MIN) ||
 	    (dj_report->device_index > DJ_DEVICE_INDEX_MAX)) {
 		dev_err(&djrcv_hdev->dev, "%s: invalid device index:%d\n",
 			__func__, dj_report->device_index);
-		return;
+		return -1;
 	}
 
 	if (djrcv_dev->paired_dj_devices[dj_report->device_index]) {
 		/* The device is already known. No need to reallocate it. */
 		dbg_hid("%s: device is already known\n", __func__);
-		return;
+		return -1;
 	}
 
 	dj_hiddev = hid_allocate_device();
 	if (IS_ERR(dj_hiddev)) {
 		dev_err(&djrcv_hdev->dev, "%s: hid_allocate_device failed\n",
 			__func__);
-		return;
+		return -1;
 	}
 
 	dj_hiddev->ll_driver = &logi_dj_ll_driver;
@@ -396,10 +397,11 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
 
 	djrcv_dev->paired_dj_devices[dj_report->device_index] = dj_dev;
 
-	return;
+	return 0;
 
 dj_device_allocate_fail:
 	hid_destroy_device(dj_hiddev);
+	return -1;
 }
 
 static void delayedwork_callback(struct work_struct *work)
@@ -441,39 +443,45 @@ static void delayedwork_callback(struct work_struct *work)
 	spin_unlock_irqrestore(&djrcv_dev->lock, flags);
 
 	switch (dj_report.report_type) {
-	case REPORT_TYPE_NOTIF_DEVICE_PAIRED:
-		logi_dj_recv_add_djhid_device(djrcv_dev, &dj_report);
-		break;
 	case REPORT_TYPE_NOTIF_DEVICE_UNPAIRED:
 		logi_dj_recv_destroy_djhid_device(djrcv_dev, &dj_report);
 		break;
+	case REPORT_TYPE_NOTIF_DEVICE_PAIRED:
+		retval = logi_dj_recv_add_djhid_device(djrcv_dev, &dj_report);
+		if (!djrcv_dev->connected[dj_report.device_index] || retval < 0)
+			break;
+		/* Device is connected, but only now paired. Fallthrough.
+		   Note: If we fallthrough here, we should not fallthrough
+		   the next case, since that's the case for connection status
+		   info arriving before paried. */
 	case REPORT_TYPE_NOTIF_CONNECTION_STATUS:
-		param_status = dj_report.report_params[
-					CONNECTION_STATUS_PARAM_STATUS];
-		connected = param_status != STATUS_LINKLOSS;
+		if (dj_report.report_type == REPORT_TYPE_NOTIF_CONNECTION_STATUS) {
+			param_status = dj_report.report_params[
+						CONNECTION_STATUS_PARAM_STATUS];
+			connected = param_status != STATUS_LINKLOSS;
+			djrcv_dev->connected[dj_report.device_index] = connected;
+		}
 		djdev = djrcv_dev->paired_dj_devices[dj_report.device_index];
 		dbg_hid("%s: got REPORT_TYPE_NOTIF_CONNECTION_STATUS %d %d\n", __func__, djdev ? djdev->hid_device_started : -1, connected);
-		if (!djdev) {
-			dev_err(&djrcv_dev->hdev->dev, "%s:"
-				"dj_dev null, unexpected device index\n",
-				__func__);
-			return;
-		}
-		if (!djdev->hid_device_started && connected) {
-			if (hid_add_device(djdev->hdev)) {
-				dev_err(&djrcv_hdev->dev,
-					"%s: failed adding dj_device\n",
-					__func__);
-			} else {
-				djdev->hid_device_started = 1;
+		if (djdev) {
+			if (!djdev->hid_device_started && connected) {
+				if (hid_add_device(djdev->hdev)) {
+					dev_err(&djrcv_hdev->dev,
+						"%s: failed adding dj_device\n",
+						__func__);
+				} else {
+					djdev->hid_device_started = 1;
+				}
 			}
+			hidpp_dev = hid_get_drvdata(djdev->hdev);
+			if (!hidpp_dev)	{
+				dbg_hid("%s: hidpp_dev is NULL\n", __func__);
+				return;
+			}
+			hidpp_connect_change(hidpp_dev, connected);
+			break;
 		}
-		hidpp_dev = hid_get_drvdata(djdev->hdev);
-		if (!hidpp_dev)	{
-			dbg_hid("%s: hidpp_dev is NULL\n", __func__);
-			return;
-		}
-		hidpp_connect_change(hidpp_dev, connected);
+		/* Fallthrough for case where djdev is NULL. */
 	default:
 	/* A normal report (i. e. not belonging to a pair/unpair notification)
 	 * arriving here, means that the report arrived but we did not have a
@@ -1012,18 +1020,12 @@ static int logi_dj_probe(struct hid_device *hdev,
 	/* Allow incoming packets to arrive: */
 	hid_device_io_start(hdev);
 
-	retval = logi_dj_recv_query_paired_devices(djrcv_dev);
-	if (retval < 0) {
-		dev_err(&hdev->dev, "%s:logi_dj_recv_query_paired_devices "
-			"error:%d\n", __func__, retval);
-		goto logi_dj_recv_query_paired_devices_failed;
-	}
+	/* Normally we'd query for paired devices here, but some controllers
+	   don't successfully send (or recieve?) the messages for this. Instead,
+	   we'll wait for packets to arrive and issue a query then. */
 
 	return retval;
 
-logi_dj_recv_query_paired_devices_failed:
-	hdev->ll_driver->close(hdev);
-
 llopen_failed:
 switch_to_dj_mode_fail:
 	hid_hw_stop(hdev);
-- 
1.8.4

