diff mbox series

[12/28] HID: logitech-dj: add logi_dj_recv_queue_unknown_work helper

Message ID 20190330112418.15042-13-hdegoede@redhat.com (mailing list archive)
State Superseded
Delegated to: Jiri Kosina
Headers show
Series HID: logitech: Handling of non DJ receivers in hid-logitech-dj | expand

Commit Message

Hans de Goede March 30, 2019, 11:24 a.m. UTC
Add a logi_dj_recv_queue_unknown_work helper and implement query
rate-limiting inside this helper.

The motivations behind this are:

1) We need to queue workitems for reports with no place to forward them
from more places with the upcoming non-unifying receiver support, hence
the addition of the helper function.

2) When we've missed a pairing info report (or there is a race between
the report and input-events) and the input report is e.g. from a mouse
being moved, we will get a lot of these before we've finished (re-)
querying and enumerating the devices, hence the rate-limiting.

Note this also removes the:

if (!djrcv_dev->paired_dj_devices[hidpp_report->device_index])

check previously guarding the sending of an unknown workitem, the caller
of logi_dj_recv_queue_notification already does this check before calling
logi_dj_recv_queue_notification.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-logitech-dj.c | 43 ++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 13 deletions(-)
diff mbox series

Patch

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 337389c2d6c7..5e65c3dfc033 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -128,6 +128,7 @@  struct dj_receiver_dev {
 	struct kref kref;
 	struct work_struct work;
 	struct kfifo notif_fifo;
+	unsigned long last_query; /* in jiffies */
 	bool ready;
 	spinlock_t lock;
 };
@@ -458,6 +459,7 @@  static struct dj_receiver_dev *dj_get_receiver_dev(struct hid_device *hdev,
 		}
 		kref_init(&djrcv_dev->kref);
 		list_add_tail(&djrcv_dev->list, &dj_hdev_list);
+		djrcv_dev->last_query = jiffies;
 	}
 
 	if (application == HID_GD_KEYBOARD)
@@ -636,6 +638,30 @@  static void delayedwork_callback(struct work_struct *work)
 	}
 }
 
+/*
+ * Sometimes we receive reports for which we do not have a paired dj_device
+ * associated with the device_index or report-type to forward the report to.
+ * This means that the original "device paired" notification corresponding
+ * to the dj_device never arrived to this driver. Possible reasons for this are:
+ * 1) hid-core discards all packets coming from a device during probe().
+ * 2) if the receiver is plugged into a KVM switch then the pairing reports
+ * are only forwarded to is if the focus is on this PC.
+ * This function deals with this by re-asking the receiver for the list of
+ * connected devices in the delayed work callback.
+ * This function MUST be called with djrcv->lock held.
+ */
+static void logi_dj_recv_queue_unknown_work(struct dj_receiver_dev *djrcv_dev)
+{
+	struct dj_workitem workitem = { .type = WORKITEM_TYPE_UNKNOWN };
+
+	/* Rate limit queries done because of unhandeled reports to 2/sec */
+	if (time_before(jiffies, djrcv_dev->last_query + HZ / 2))
+		return;
+
+	kfifo_in(&djrcv_dev->notif_fifo, &workitem, sizeof(workitem));
+	schedule_work(&djrcv_dev->work);
+}
+
 static void logi_dj_recv_queue_notification(struct dj_receiver_dev *djrcv_dev,
 					   struct dj_report *dj_report)
 {
@@ -665,19 +691,8 @@  static void logi_dj_recv_queue_notification(struct dj_receiver_dev *djrcv_dev,
 			workitem.type = WORKITEM_TYPE_UNPAIRED;
 		break;
 	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
-	 * paired dj_device associated to the report's device_index, this
-	 * means that the original "device paired" notification corresponding
-	 * to this dj_device never arrived to this driver. The reason is that
-	 * hid-core discards all packets coming from a device while probe() is
-	 * executing. */
-		if (!djrcv_dev->paired_dj_devices[dj_report->device_index]) {
-			/* ok, we don't know the device, just re-ask the
-			 * receiver for the list of connected devices in
-			 * the delayed work callback. */
-			workitem.type = WORKITEM_TYPE_UNKNOWN;
-		}
+		logi_dj_recv_queue_unknown_work(djrcv_dev);
+		return;
 	}
 
 	kfifo_in(&djrcv_dev->notif_fifo, &workitem, sizeof(workitem));
@@ -773,6 +788,8 @@  static int logi_dj_recv_query_paired_devices(struct dj_receiver_dev *djrcv_dev)
 	struct dj_report *dj_report;
 	int retval;
 
+	djrcv_dev->last_query = jiffies;
+
 	dj_report = kzalloc(sizeof(struct dj_report), GFP_KERNEL);
 	if (!dj_report)
 		return -ENOMEM;