Changeset 970f6e1 in mainline

Timestamp:

2018-01-09T18:25:56Z (6 years ago)

Author:

Petr Manek <petr.manek@…>

Branches:

lfn, master, serial, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

4793023

Parents:

c386d6d

Message:

usbhid: join polling fibrils in device_remove()

Location:

uspace/drv/hid/usbhid

Files:

• generic/hiddev.c (modified) (1 diff)
• main.c (modified) (4 diffs)
• mouse/mousedev.c (modified) (7 diffs)
• usbhid.c (modified) (7 diffs)
• usbhid.h (modified) (3 diffs)

uspace/drv/hid/usbhid/generic/hiddev.c

@@ -219 +219 @@
 bool usb_generic_hid_polling_callback(usb_hid_dev_t *hid_dev, void *data)
 {
-        return true;
+        /* Continue polling until the device is about to be removed. */
+        return !hid_dev->will_deinit;
 }
 

uspace/drv/hid/usbhid/main.c

@@ -40 +40 @@
 #include <errno.h>
 #include <str_error.h>
+#include <fibril_synch.h>
 
 #include <usb/dev/driver.h>
@@ -98 +99 @@
            /* Delay */
            -1,
+           /* Callback when the polling fails. */
+           usb_hid_polling_error_callback,
            /* Callback when the polling ends. */
            usb_hid_polling_ended_callback,
@@ -128 +131 @@
         assert(hid_dev);
 
-        /* TODO: Stop device polling prior to deinit. Now it fails on endpoint error. */
-
+        usb_log_debug2("%s will be removed, setting remove flag.\n", usb_device_get_name(dev));
+        usb_hid_prepare_deinit(hid_dev);
+
+        return EOK;
+}
+
+/**
+ * Callback for when a device has just been from the driver.
+ *
+ * @param dev Structure representing the device.
+ * @return Error code.
+ */
+static int usb_hid_device_removed(usb_device_t *dev)
+{
+        assert(dev);
+        usb_hid_dev_t *hid_dev = usb_device_data_get(dev);
+        assert(hid_dev);
+
+        /* Join polling fibril. */
+        fibril_mutex_lock(&hid_dev->guard);
+        while (hid_dev->running)
+                fibril_condvar_wait(&hid_dev->poll_end, &hid_dev->guard);
+        fibril_mutex_unlock(&hid_dev->guard);
+
+        /* Clean up. */
         usb_hid_deinit(hid_dev);
         usb_log_debug2("%s destruction complete.\n", usb_device_get_name(dev));
+
         return EOK;
 }
@@ -165 +192 @@
         .device_add = usb_hid_device_add,
         .device_remove = usb_hid_device_remove,
+        .device_removed = usb_hid_device_removed,
         .device_gone = usb_hid_device_gone,
 };

uspace/drv/hid/usbhid/mouse/mousedev.c

@@ -165 +165 @@
 }
 
-static bool usb_mouse_process_report(usb_hid_dev_t *hid_dev,
+static void usb_mouse_process_report(usb_hid_dev_t *hid_dev,
     usb_mouse_t *mouse_dev)
 {
@@ -172 +172 @@
         if (mouse_dev->mouse_sess == NULL) {
                 usb_log_warning(NAME " No console session.\n");
-                return true;
+                return;
         }
 
@@ -191 +191 @@
         if (absolute_x != absolute_y) {
                 usb_log_error(NAME " cannot handle mix of absolute and relative mouse move.");
-                return true;
+                return;
         }
 
@@ -226 +226 @@
         if (path == NULL) {
                 usb_log_warning("Failed to create USB HID report path.\n");
-                return true;
+                return;
         }
         int ret =
@@ -233 +233 @@
                 usb_hid_report_path_free(path);
                 usb_log_warning("Failed to add buttons to report path.\n");
-                return true;
+                return;
         }
         usb_hid_report_path_set_report_id(path, hid_dev->report_id);
@@ -266 +266 @@
 
         usb_hid_report_path_free(path);
-
-        return true;
 }
 
@@ -414 +412 @@
 
         usb_mouse_t *mouse_dev = data;
-
-        return usb_mouse_process_report(hid_dev, mouse_dev);
+        usb_mouse_process_report(hid_dev, mouse_dev);
+
+        /* Continue polling until the device is about to be removed. */
+        return !hid_dev->will_deinit;
 }
 

uspace/drv/hid/usbhid/usbhid.c

@@ -332 +332 @@
  * During initialization, the keyboard is switched into boot protocol, the idle
  * rate is set to 0 (infinity), resulting in the keyboard only reporting event
- * when a key is pressed or released. Finally, the LED lights are turned on 
+ * when a key is pressed or released. Finally, the LED lights are turned on
  * according to the default setup of lock keys.
  *
- * @note By default, the keyboards is initialized with Num Lock turned on and 
+ * @note By default, the keyboards is initialized with Num Lock turned on and
  *       other locks turned off.
  *
@@ -354 +354 @@
         hid_dev->usb_dev = dev;
         hid_dev->poll_pipe_mapping = NULL;
+
+        hid_dev->will_deinit = false;
+        fibril_mutex_initialize(&hid_dev->guard);
+        fibril_condvar_initialize(&hid_dev->poll_end);
 
         int rc = usb_hid_check_pipes(hid_dev, dev);
@@ -492 +496 @@
 }
 
+bool usb_hid_polling_error_callback(usb_device_t *dev, int err_code, void *arg)
+{
+        assert(dev);
+        assert(arg);
+        usb_hid_dev_t *hid_dev = arg;
+
+        usb_log_error("Device %s polling error: %s", usb_device_get_name(dev),
+            str_error(err_code));
+
+        /* Continue polling until the device is about to be removed. */
+        return hid_dev->running && !hid_dev->will_deinit;
+}
+
 void usb_hid_polling_ended_callback(usb_device_t *dev, bool reason, void *arg)
 {
@@ -507 +524 @@
 
         hid_dev->running = false;
+
+        /* Signal polling end to joining thread. */
+        fibril_mutex_lock(&hid_dev->guard);
+        fibril_condvar_signal(&hid_dev->poll_end);
+        fibril_mutex_unlock(&hid_dev->guard);
 }
 
@@ -517 +539 @@
 {
         return hid_dev->report_nr;
+}
+
+void usb_hid_prepare_deinit(usb_hid_dev_t *hid_dev)
+{
+        assert(hid_dev);
+        hid_dev->will_deinit = true;
 }
 
@@ -524 +552 @@
         assert(hid_dev->subdrivers != NULL || hid_dev->subdriver_count == 0);
 
-
-        usb_log_debug("Subdrivers: %p, subdriver count: %d\n", 
+        usb_log_debug("Subdrivers: %p, subdriver count: %d\n",
             hid_dev->subdrivers, hid_dev->subdriver_count);
 
@@ -541 +568 @@
         /* Destroy the parser */
         usb_hid_report_deinit(&hid_dev->report);
-
 }
 

uspace/drv/hid/usbhid/usbhid.h

@@ -45 +45 @@
 #include <usb/hid/hid.h>
 #include <stdbool.h>
+#include <fibril_synch.h>
 
 typedef struct usb_hid_dev usb_hid_dev_t;
@@ -130 +131 @@
         int report_nr;
         volatile bool running;
+
+        volatile bool will_deinit;
+        fibril_mutex_t guard;
+        fibril_condvar_t poll_end;
 };
 
@@ -136 +141 @@
 int usb_hid_init(usb_hid_dev_t *hid_dev, usb_device_t *dev);
 
+void usb_hid_prepare_deinit(usb_hid_dev_t *hid_dev);
+
 void usb_hid_deinit(usb_hid_dev_t *hid_dev);
 
 bool usb_hid_polling_callback(usb_device_t *dev,
     uint8_t *buffer, size_t buffer_size, void *arg);
+
+bool usb_hid_polling_error_callback(usb_device_t *dev, int err_code, void *arg);
 
 void usb_hid_polling_ended_callback(usb_device_t *dev, bool reason, void *arg);