HID: logitech-dj: do not schedule the dj report itself

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: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 23d3e03..628c3bd 100644 (file)
--- 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__);