HID: Separate struct hid_device's driver_lock into two locks.

This patch separates struct hid_device's driver_lock into two. The
goal is to allow hid device drivers to receive input during their
probe() or remove() function calls. This is necessary because some
drivers need to communicate with the device to determine parameters
needed during probe (e.g., size of a multi-touch surface), and if
possible, may perfer to communicate with a device on host-initiated
disconnect (e.g., to put it into a low-power state).

Historically, three functions used driver_lock:

- hid_device_probe: blocks to acquire lock
- hid_device_remove: blocks to acquire lock
- hid_input_report: if locked returns -EBUSY, else acquires lock

This patch adds another lock (driver_input_lock) which is used to
block input from occurring. The lock behavior is now:

- hid_device_probe: blocks to acq. driver_lock, then driver_input_lock
- hid_device_remove: blocks to acq. driver_lock, then driver_input_lock
- hid_input_report: if driver_input_lock locked returns -EBUSY, else
  acquires driver_input_lock

This patch also adds two helper functions to be called during probe()
or remove(): hid_device_io_start() and hid_device_io_stop(). These
functions lock and unlock, respectively, driver_input_lock; they also
make a note of whether they did so that hid-core knows if a driver has
changed the lock state.

This patch results in no behavior change for existing devices and
drivers. However, during a probe() or remove() function call in a
driver, that driver may now selectively call hid_device_io_start() to
let input events come through, then optionally call
hid_device_io_stop() to stop them.

Signed-off-by: Andrew de los Reyes <adlr@chromium.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index ff75cabf7393f3d29c8b05c77f11cf2b8828a132..680068c0c46af3ca8f9d61bf5d987a86e24c77eb 100644 (file)
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1267,7 +1267,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int i
        if (!hid)
                return -ENODEV;
 
-       if (down_trylock(&hid->driver_lock))
+       if (down_trylock(&hid->driver_input_lock))
                return -EBUSY;
 
        if (!hid->driver) {
@@ -1324,7 +1324,7 @@ nomem:
        ret = hid_report_raw_event(hid, type, data, size, interrupt);
 
 unlock:
-       up(&hid->driver_lock);
+       up(&hid->driver_input_lock);
        return ret;
 }
 EXPORT_SYMBOL_GPL(hid_input_report);
@@ -1845,6 +1845,11 @@ static int hid_device_probe(struct device *dev)
 
        if (down_interruptible(&hdev->driver_lock))
                return -EINTR;
+       if (down_interruptible(&hdev->driver_input_lock)) {
+               ret = -EINTR;
+               goto unlock_driver_lock;
+       }
+       hdev->io_started = false;
 
        if (!hdev->driver) {
                id = hid_match_device(hdev, hdrv);
@@ -1867,6 +1872,9 @@ static int hid_device_probe(struct device *dev)
                }
        }
 unlock:
+       if (!hdev->io_started)
+               up(&hdev->driver_input_lock);
+unlock_driver_lock:
        up(&hdev->driver_lock);
        return ret;
 }
@@ -1875,9 +1883,15 @@ static int hid_device_remove(struct device *dev)
 {
        struct hid_device *hdev = container_of(dev, struct hid_device, dev);
        struct hid_driver *hdrv;
+       int ret = 0;
 
        if (down_interruptible(&hdev->driver_lock))
                return -EINTR;
+       if (down_interruptible(&hdev->driver_input_lock)) {
+               ret = -EINTR;
+               goto unlock_driver_lock;
+       }
+       hdev->io_started = false;
 
        hdrv = hdev->driver;
        if (hdrv) {
@@ -1889,8 +1903,11 @@ static int hid_device_remove(struct device *dev)
                hdev->driver = NULL;
        }
 
+       if (!hdev->io_started)
+               up(&hdev->driver_input_lock);
+unlock_driver_lock:
        up(&hdev->driver_lock);
-       return 0;
+       return ret;
 }
 
 static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
@@ -2329,6 +2346,7 @@ struct hid_device *hid_allocate_device(void)
        init_waitqueue_head(&hdev->debug_wait);
        INIT_LIST_HEAD(&hdev->debug_list);
        sema_init(&hdev->driver_lock, 1);
+       sema_init(&hdev->driver_input_lock, 1);
 
        return hdev;
 }

diff --git a/include/linux/hid.h b/include/linux/hid.h
index e14b465b11463d353553027e602d79499d8be01d..895b85639dec6a1f7d022df14024fe5ae56e6db1 100644 (file)
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -456,7 +456,8 @@ struct hid_device {                                                 /* device report descriptor */
        unsigned country;                                               /* HID country */
        struct hid_report_enum report_enum[HID_REPORT_TYPES];
 
-       struct semaphore driver_lock;                                   /* protects the current driver */
+       struct semaphore driver_lock;                                   /* protects the current driver, except during input */
+       struct semaphore driver_input_lock;                             /* protects the current driver */
        struct device dev;                                              /* device */
        struct hid_driver *driver;
        struct hid_ll_driver *ll_driver;
@@ -477,6 +478,7 @@ struct hid_device {                                                 /* device report descriptor */
        unsigned int status;                                            /* see STAT flags above */
        unsigned claimed;                                               /* Claimed by hidinput, hiddev? */
        unsigned quirks;                                                /* Various quirks the device can pull on us */
+       bool io_started;                                                /* Protected by driver_lock. If IO has started */
 
        struct list_head inputs;                                        /* The list of inputs */
        void *hiddev;                                                   /* The hiddev structure */
@@ -599,6 +601,10 @@ struct hid_usage_id {
  * @resume: invoked on resume if device was not reset (NULL means nop)
  * @reset_resume: invoked on resume if device was reset (NULL means nop)
  *
+ * probe should return -errno on error, or 0 on success. During probe,
+ * input will not be passed to raw_event unless hid_device_io_start is
+ * called.
+ *
  * raw_event and event should return 0 on no action performed, 1 when no
  * further processing should be done and negative on error
  *
@@ -737,6 +743,44 @@ const struct hid_device_id *hid_match_id(struct hid_device *hdev,
                                         const struct hid_device_id *id);
 s32 hid_snto32(__u32 value, unsigned n);
 
+/**
+ * hid_device_io_start - enable HID input during probe, remove
+ *
+ * @hid - the device
+ *
+ * This should only be called during probe or remove and only be
+ * called by the thread calling probe or remove. It will allow
+ * incoming packets to be delivered to the driver.
+ */
+static inline void hid_device_io_start(struct hid_device *hid) {
+       if (hid->io_started) {
+               dev_warn(&hid->dev, "io already started");
+               return;
+       }
+       hid->io_started = true;
+       up(&hid->driver_input_lock);
+}
+
+/**
+ * hid_device_io_stop - disable HID input during probe, remove
+ *
+ * @hid - the device
+ *
+ * Should only be called after hid_device_io_start. It will prevent
+ * incoming packets from going to the driver for the duration of
+ * probe, remove. If called during probe, packets will still go to the
+ * driver after probe is complete. This function should only be called
+ * by the thread calling probe or remove.
+ */
+static inline void hid_device_io_stop(struct hid_device *hid) {
+       if (!hid->io_started) {
+               dev_warn(&hid->dev, "io already stopped");
+               return;
+       }
+       hid->io_started = false;
+       down(&hid->driver_input_lock);
+}
+
 /**
  * hid_map_usage - map usage input bits
  *