2023-05-23 21:09:38

by Dmitry Torokhov

[permalink] [raw]
Subject: [PATCH v2 1/2] HID: split apart hid_device_probe to make logic more apparent

hid_device_probe() has a complex flow and locks and unlocks a mutex.
Move the most of the logic into __hid_device_probe() and
hid_check_device_match() and leave the locking in hid_device_probe()
which makes the code more clear.

Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/hid/hid-core.c | 101 ++++++++++++++++++++++++++-----------------------
1 file changed, 54 insertions(+), 47 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 22623eb4f72f..1f0bb2784bfc 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2587,64 +2587,71 @@ bool hid_compare_device_paths(struct hid_device *hdev_a,
}
EXPORT_SYMBOL_GPL(hid_compare_device_paths);

+static bool hid_check_device_match(struct hid_device *hdev,
+ struct hid_driver *hdrv,
+ const struct hid_device_id **id)
+{
+ *id = hid_match_device(hdev, hdrv);
+ if (!*id)
+ return -ENODEV;
+
+ if (hdrv->match)
+ return hdrv->match(hdev, hid_ignore_special_drivers);
+
+ /*
+ * hid-generic implements .match(), so we must be dealing with a
+ * different HID driver here, and can simply check if
+ * hid_ignore_special_drivers is set or not.
+ */
+ return !hid_ignore_special_drivers;
+}
+
+static int __hid_device_probe(struct hid_device *hdev, struct hid_driver *hdrv)
+{
+ const struct hid_device_id *id;
+ int ret;
+
+ if (!hid_check_device_match(hdev, hdrv, &id))
+ return -ENODEV;
+
+ /* reset the quirks that has been previously set */
+ hdev->quirks = hid_lookup_quirk(hdev);
+ hdev->driver = hdrv;
+
+ if (hdrv->probe) {
+ ret = hdrv->probe(hdev, id);
+ } else { /* default probe */
+ ret = hid_open_report(hdev);
+ if (!ret)
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ }
+
+ if (ret) {
+ hid_close_report(hdev);
+ hdev->driver = NULL;
+ }
+
+ return ret;
+}
+
static int hid_device_probe(struct device *dev)
{
- struct hid_driver *hdrv = to_hid_driver(dev->driver);
struct hid_device *hdev = to_hid_device(dev);
- const struct hid_device_id *id;
+ struct hid_driver *hdrv = to_hid_driver(dev->driver);
int ret = 0;

- if (down_interruptible(&hdev->driver_input_lock)) {
- ret = -EINTR;
- goto end;
- }
- hdev->io_started = false;
+ if (down_interruptible(&hdev->driver_input_lock))
+ return -EINTR;

+ hdev->io_started = false;
clear_bit(ffs(HID_STAT_REPROBED), &hdev->status);

- if (!hdev->driver) {
- id = hid_match_device(hdev, hdrv);
- if (id == NULL) {
- ret = -ENODEV;
- goto unlock;
- }
+ if (!hdev->driver)
+ ret = __hid_device_probe(hdev, hdrv);

- if (hdrv->match) {
- if (!hdrv->match(hdev, hid_ignore_special_drivers)) {
- ret = -ENODEV;
- goto unlock;
- }
- } else {
- /*
- * hid-generic implements .match(), so if
- * hid_ignore_special_drivers is set, we can safely
- * return.
- */
- if (hid_ignore_special_drivers) {
- ret = -ENODEV;
- goto unlock;
- }
- }
-
- /* reset the quirks that has been previously set */
- hdev->quirks = hid_lookup_quirk(hdev);
- hdev->driver = hdrv;
- if (hdrv->probe) {
- ret = hdrv->probe(hdev, id);
- } else { /* default probe */
- ret = hid_open_report(hdev);
- if (!ret)
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
- }
- if (ret) {
- hid_close_report(hdev);
- hdev->driver = NULL;
- }
- }
-unlock:
if (!hdev->io_started)
up(&hdev->driver_input_lock);
-end:
+
return ret;
}


--
2.40.1.698.g37aff9b760-goog