diff mbox series

[v3,08/37] HID: logitech-dj: do not schedule the dj report itself

Message ID 20190420112217.27590-9-hdegoede@redhat.com (mailing list archive)
State Mainlined
Commit 4fcad95a24bba1601c15b308b97fa7a1090135cd
Delegated to: Jiri Kosina
Headers show
Series HID: logitech: Handling of non DJ receivers in hid-logitech-dj | expand

Commit Message

Hans de Goede April 20, 2019, 11:21 a.m. UTC
From: Benjamin Tissoires <benjamin.tissoires@redhat.com>

This is a preparatory patch for handling non DJ (HID++ only) receivers,
through this module. We can not use the dj_report in the delayed work
callback as the HID++ notifications are different both in size and meaning.

There should be no functional change.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3 (HdG):
-Fix style of some multi-line comments
---
 drivers/hid/hid-logitech-dj.c | 136 +++++++++++++++++++++-------------
 1 file changed, 85 insertions(+), 51 deletions(-)
diff mbox series

Patch

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 23d3e039ebbb..628c3bd87a0d 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -31,7 +31,7 @@ 
 #include "hid-ids.h"
 
 #define DJ_MAX_PAIRED_DEVICES			6
-#define DJ_MAX_NUMBER_NOTIFICATIONS		8
+#define DJ_MAX_NUMBER_NOTIFS			8
 #define DJ_RECEIVER_INDEX			0
 #define DJ_DEVICE_INDEX_MIN			1
 #define DJ_DEVICE_INDEX_MAX			6
@@ -135,6 +135,19 @@  struct dj_device {
 	u8 device_index;
 };
 
+#define WORKITEM_TYPE_EMPTY	0
+#define WORKITEM_TYPE_PAIRED	1
+#define WORKITEM_TYPE_UNPAIRED	2
+#define WORKITEM_TYPE_UNKNOWN	255
+
+struct dj_workitem {
+	u8 type;		/* WORKITEM_TYPE_* */
+	u8 device_index;
+	u8 quad_id_msb;
+	u8 quad_id_lsb;
+	u32 reports_supported;
+};
+
 /* Keyboard descriptor (1) */
 static const char kbd_descriptor[] = {
 	0x05, 0x01,		/* USAGE_PAGE (generic Desktop)     */
@@ -353,15 +366,15 @@  static struct hid_ll_driver logi_dj_ll_driver;
 static int logi_dj_recv_query_paired_devices(struct dj_receiver_dev *djrcv_dev);
 
 static void logi_dj_recv_destroy_djhid_device(struct dj_receiver_dev *djrcv_dev,
-						struct dj_report *dj_report)
+					      struct dj_workitem *workitem)
 {
 	/* Called in delayed work context */
 	struct dj_device *dj_dev;
 	unsigned long flags;
 
 	spin_lock_irqsave(&djrcv_dev->lock, flags);
-	dj_dev = djrcv_dev->paired_dj_devices[dj_report->device_index];
-	djrcv_dev->paired_dj_devices[dj_report->device_index] = NULL;
+	dj_dev = djrcv_dev->paired_dj_devices[workitem->device_index];
+	djrcv_dev->paired_dj_devices[workitem->device_index] = NULL;
 	spin_unlock_irqrestore(&djrcv_dev->lock, flags);
 
 	if (dj_dev != NULL) {
@@ -374,26 +387,20 @@  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)
+					  struct dj_workitem *workitem)
 {
 	/* Called in delayed work context */
 	struct hid_device *djrcv_hdev = djrcv_dev->hdev;
 	struct hid_device *dj_hiddev;
 	struct dj_device *dj_dev;
+	u8 device_index = workitem->device_index;
 
 	/* Device index goes from 1 to 6, we need 3 bytes to store the
 	 * semicolon, the index, and a null terminator
 	 */
 	unsigned char tmpstr[3];
 
-	if (dj_report->report_params[DEVICE_PAIRED_PARAM_SPFUNCTION] &
-	    SPFUNCTION_DEVICE_LIST_EMPTY) {
-		dbg_hid("%s: device list is empty\n", __func__);
-		djrcv_dev->querying_devices = false;
-		return;
-	}
-
-	if (djrcv_dev->paired_dj_devices[dj_report->device_index]) {
+	if (djrcv_dev->paired_dj_devices[device_index]) {
 		/* The device is already known. No need to reallocate it. */
 		dbg_hid("%s: device is already known\n", __func__);
 		return;
@@ -411,10 +418,8 @@  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 = djrcv_hdev->vendor;
-	dj_hiddev->product =
-		(dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_MSB]
-									<< 8) |
-		dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_LSB];
+	dj_hiddev->product = (workitem->quad_id_msb << 8) |
+			      workitem->quad_id_lsb;
 	snprintf(dj_hiddev->name, sizeof(dj_hiddev->name),
 		"Logitech Unifying Device. Wireless PID:%04x",
 		dj_hiddev->product);
@@ -422,7 +427,7 @@  static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
 	dj_hiddev->group = HID_GROUP_LOGITECH_DJ_DEVICE;
 
 	memcpy(dj_hiddev->phys, djrcv_hdev->phys, sizeof(djrcv_hdev->phys));
-	snprintf(tmpstr, sizeof(tmpstr), ":%d", dj_report->device_index);
+	snprintf(tmpstr, sizeof(tmpstr), ":%d", device_index);
 	strlcat(dj_hiddev->phys, tmpstr, sizeof(dj_hiddev->phys));
 
 	dj_dev = kzalloc(sizeof(struct dj_device), GFP_KERNEL);
@@ -433,14 +438,13 @@  static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
 		goto dj_device_allocate_fail;
 	}
 
-	dj_dev->reports_supported = get_unaligned_le32(
-		dj_report->report_params + DEVICE_PAIRED_RF_REPORT_TYPE);
+	dj_dev->reports_supported = workitem->reports_supported;
 	dj_dev->hdev = dj_hiddev;
 	dj_dev->dj_receiver_dev = djrcv_dev;
-	dj_dev->device_index = dj_report->device_index;
+	dj_dev->device_index = device_index;
 	dj_hiddev->driver_data = dj_dev;
 
-	djrcv_dev->paired_dj_devices[dj_report->device_index] = dj_dev;
+	djrcv_dev->paired_dj_devices[device_index] = dj_dev;
 
 	if (hid_add_device(dj_hiddev)) {
 		dev_err(&djrcv_hdev->dev, "%s: failed adding dj_device\n",
@@ -451,7 +455,7 @@  static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
 	return;
 
 hid_add_device_fail:
-	djrcv_dev->paired_dj_devices[dj_report->device_index] = NULL;
+	djrcv_dev->paired_dj_devices[device_index] = NULL;
 	kfree(dj_dev);
 dj_device_allocate_fail:
 	hid_destroy_device(dj_hiddev);
@@ -462,7 +466,7 @@  static void delayedwork_callback(struct work_struct *work)
 	struct dj_receiver_dev *djrcv_dev =
 		container_of(work, struct dj_receiver_dev, work);
 
-	struct dj_report dj_report;
+	struct dj_workitem workitem;
 	unsigned long flags;
 	int count;
 	int retval;
@@ -471,10 +475,9 @@  static void delayedwork_callback(struct work_struct *work)
 
 	spin_lock_irqsave(&djrcv_dev->lock, flags);
 
-	count = kfifo_out(&djrcv_dev->notif_fifo, &dj_report,
-				sizeof(struct dj_report));
+	count = kfifo_out(&djrcv_dev->notif_fifo, &workitem, sizeof(workitem));
 
-	if (count != sizeof(struct dj_report)) {
+	if (count != sizeof(workitem)) {
 		dev_err(&djrcv_dev->hdev->dev, "%s: workitem triggered without "
 			"notifications available\n", __func__);
 		spin_unlock_irqrestore(&djrcv_dev->lock, flags);
@@ -490,12 +493,54 @@  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);
+	switch (workitem.type) {
+	case WORKITEM_TYPE_PAIRED:
+		logi_dj_recv_add_djhid_device(djrcv_dev, &workitem);
 		break;
+	case WORKITEM_TYPE_UNPAIRED:
+		logi_dj_recv_destroy_djhid_device(djrcv_dev, &workitem);
+		break;
+	case WORKITEM_TYPE_UNKNOWN:
+		retval = logi_dj_recv_query_paired_devices(djrcv_dev);
+		if (retval) {
+			dev_err(&djrcv_dev->hdev->dev,
+				"%s: logi_dj_recv_query_paired_devices error: %d\n",
+				__func__, retval);
+		}
+		break;
+	case WORKITEM_TYPE_EMPTY:
+		dbg_hid("%s: device list is empty\n", __func__);
+		break;
+	}
+}
+
+static void logi_dj_recv_queue_notification(struct dj_receiver_dev *djrcv_dev,
+					   struct dj_report *dj_report)
+{
+	/* We are called from atomic context (tasklet && djrcv->lock held) */
+	struct dj_workitem workitem = {
+		.device_index = dj_report->device_index,
+	};
+
+	switch (dj_report->report_type) {
+	case REPORT_TYPE_NOTIF_DEVICE_PAIRED:
+		workitem.type = WORKITEM_TYPE_PAIRED;
+		if (dj_report->report_params[DEVICE_PAIRED_PARAM_SPFUNCTION] &
+		    SPFUNCTION_DEVICE_LIST_EMPTY) {
+			workitem.type = WORKITEM_TYPE_EMPTY;
+			break;
+		}
+		/* fall-through */
 	case REPORT_TYPE_NOTIF_DEVICE_UNPAIRED:
-		logi_dj_recv_destroy_djhid_device(djrcv_dev, &dj_report);
+		workitem.quad_id_msb =
+			dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_MSB];
+		workitem.quad_id_lsb =
+			dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_LSB];
+		workitem.reports_supported = get_unaligned_le32(
+						dj_report->report_params +
+						DEVICE_PAIRED_RF_REPORT_TYPE);
+		if (dj_report->report_type == REPORT_TYPE_NOTIF_DEVICE_UNPAIRED)
+			workitem.type = WORKITEM_TYPE_UNPAIRED;
 		break;
 	default:
 	/* A normal report (i. e. not belonging to a pair/unpair notification)
@@ -505,28 +550,17 @@  static void delayedwork_callback(struct work_struct *work)
 	 * 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. */
-		retval = logi_dj_recv_query_paired_devices(djrcv_dev);
-		if (!retval) {
-			/* everything went fine, so just leave */
-			break;
-		}
-		dev_err(&djrcv_dev->hdev->dev,
-			"%s:logi_dj_recv_query_paired_devices "
-			"error:%d\n", __func__, retval);
+		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;
 		}
-		dbg_hid("%s: unexpected report type\n", __func__);
 	}
-}
-
-static void logi_dj_recv_queue_notification(struct dj_receiver_dev *djrcv_dev,
-					   struct dj_report *dj_report)
-{
-	/* We are called from atomic context (tasklet && djrcv->lock held) */
 
-	kfifo_in(&djrcv_dev->notif_fifo, dj_report, sizeof(struct dj_report));
+	kfifo_in(&djrcv_dev->notif_fifo, &workitem, sizeof(workitem));
 
 	if (schedule_work(&djrcv_dev->work) == 0) {
 		dbg_hid("%s: did not schedule the work item, was already "
@@ -1055,7 +1089,7 @@  static int logi_dj_probe(struct hid_device *hdev,
 	INIT_WORK(&djrcv_dev->work, delayedwork_callback);
 	spin_lock_init(&djrcv_dev->lock);
 	if (kfifo_alloc(&djrcv_dev->notif_fifo,
-			DJ_MAX_NUMBER_NOTIFICATIONS * sizeof(struct dj_report),
+			DJ_MAX_NUMBER_NOTIFS * sizeof(struct dj_workitem),
 			GFP_KERNEL)) {
 		dev_err(&hdev->dev,
 			"%s:failed allocating notif_fifo\n", __func__);