HID: i2c-hid: Rearrange probe() to power things up later
authorDouglas Anderson <dianders@chromium.org>
Thu, 27 Jul 2023 17:16:33 +0000 (10:16 -0700)
committerDouglas Anderson <dianders@chromium.org>
Tue, 1 Aug 2023 14:39:44 +0000 (07:39 -0700)
In a future patch, we want to change i2c-hid not to necessarily power
up the touchscreen during probe. In preparation for that, rearrange
the probe function so that we put as much stuff _before_ powering up
the device as possible.

This change is expected to have no functional effect.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
Reviewed-by: Benjamin Tissoires <bentiss@kernel.org>
Acked-by: Benjamin Tissoires <bentiss@kernel.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20230727101636.v4.6.Ifcc9b0a44895d164788966f9b9511fe094ca8cf9@changeid
drivers/hid/i2c-hid/i2c-hid-core.c

index 19d985c..d29e642 100644 (file)
@@ -855,7 +855,8 @@ static int i2c_hid_init_irq(struct i2c_client *client)
                irqflags = IRQF_TRIGGER_LOW;
 
        ret = request_threaded_irq(client->irq, NULL, i2c_hid_irq,
-                                  irqflags | IRQF_ONESHOT, client->name, ihid);
+                                  irqflags | IRQF_ONESHOT | IRQF_NO_AUTOEN,
+                                  client->name, ihid);
        if (ret < 0) {
                dev_warn(&client->dev,
                        "Could not register for %s interrupt, irq = %d,"
@@ -940,6 +941,72 @@ static void i2c_hid_core_shutdown_tail(struct i2c_hid *ihid)
        ihid->ops->shutdown_tail(ihid->ops);
 }
 
+/**
+ * i2c_hid_core_initial_power_up() - First time power up of the i2c-hid device.
+ * @ihid: The ihid object created during probe.
+ *
+ * This function is called at probe time.
+ *
+ * The initial power on is where we do some basic validation that the device
+ * exists, where we fetch the HID descriptor, and where we create the actual
+ * HID devices.
+ *
+ * Return: 0 or error code.
+ */
+static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
+{
+       struct i2c_client *client = ihid->client;
+       struct hid_device *hid = ihid->hid;
+       int ret;
+
+       ret = i2c_hid_core_power_up(ihid);
+       if (ret)
+               return ret;
+
+       /* Make sure there is something at this address */
+       ret = i2c_smbus_read_byte(client);
+       if (ret < 0) {
+               i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret);
+               ret = -ENXIO;
+               goto err;
+       }
+
+       ret = i2c_hid_fetch_hid_descriptor(ihid);
+       if (ret < 0) {
+               dev_err(&client->dev,
+                       "Failed to fetch the HID Descriptor\n");
+               goto err;
+       }
+
+       enable_irq(client->irq);
+
+       hid->version = le16_to_cpu(ihid->hdesc.bcdVersion);
+       hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
+       hid->product = le16_to_cpu(ihid->hdesc.wProductID);
+
+       hid->initial_quirks |= i2c_hid_get_dmi_quirks(hid->vendor,
+                                                     hid->product);
+
+       snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X",
+                client->name, (u16)hid->vendor, (u16)hid->product);
+       strscpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys));
+
+       ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
+
+       ret = hid_add_device(hid);
+       if (ret) {
+               if (ret != -ENODEV)
+                       hid_err(client, "can't add hid device: %d\n", ret);
+               goto err;
+       }
+
+       return 0;
+
+err:
+       i2c_hid_core_power_down(ihid);
+       return ret;
+}
+
 int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
                       u16 hid_descriptor_address, u32 quirks)
 {
@@ -966,16 +1033,10 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
        if (!ihid)
                return -ENOMEM;
 
-       ihid->ops = ops;
-
-       ret = i2c_hid_core_power_up(ihid);
-       if (ret)
-               return ret;
-
        i2c_set_clientdata(client, ihid);
 
+       ihid->ops = ops;
        ihid->client = client;
-
        ihid->wHIDDescRegister = cpu_to_le16(hid_descriptor_address);
 
        init_waitqueue_head(&ihid->wait);
@@ -986,28 +1047,12 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
         * real computation later. */
        ret = i2c_hid_alloc_buffers(ihid, HID_MIN_BUFFER_SIZE);
        if (ret < 0)
-               goto err_powered;
-
+               return ret;
        device_enable_async_suspend(&client->dev);
 
-       /* Make sure there is something at this address */
-       ret = i2c_smbus_read_byte(client);
-       if (ret < 0) {
-               i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret);
-               ret = -ENXIO;
-               goto err_powered;
-       }
-
-       ret = i2c_hid_fetch_hid_descriptor(ihid);
-       if (ret < 0) {
-               dev_err(&client->dev,
-                       "Failed to fetch the HID Descriptor\n");
-               goto err_powered;
-       }
-
        ret = i2c_hid_init_irq(client);
        if (ret < 0)
-               goto err_powered;
+               goto err_buffers_allocated;
 
        hid = hid_allocate_device();
        if (IS_ERR(hid)) {
@@ -1021,26 +1066,11 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
        hid->ll_driver = &i2c_hid_ll_driver;
        hid->dev.parent = &client->dev;
        hid->bus = BUS_I2C;
-       hid->version = le16_to_cpu(ihid->hdesc.bcdVersion);
-       hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
-       hid->product = le16_to_cpu(ihid->hdesc.wProductID);
-
        hid->initial_quirks = quirks;
-       hid->initial_quirks |= i2c_hid_get_dmi_quirks(hid->vendor,
-                                                     hid->product);
-
-       snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X",
-                client->name, (u16)hid->vendor, (u16)hid->product);
-       strscpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys));
-
-       ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
 
-       ret = hid_add_device(hid);
-       if (ret) {
-               if (ret != -ENODEV)
-                       hid_err(client, "can't add hid device: %d\n", ret);
+       ret = i2c_hid_core_initial_power_up(ihid);
+       if (ret)
                goto err_mem_free;
-       }
 
        return 0;
 
@@ -1050,9 +1080,9 @@ err_mem_free:
 err_irq:
        free_irq(client->irq, ihid);
 
-err_powered:
-       i2c_hid_core_power_down(ihid);
+err_buffers_allocated:
        i2c_hid_free_buffers(ihid);
+
        return ret;
 }
 EXPORT_SYMBOL_GPL(i2c_hid_core_probe);
@@ -1062,6 +1092,8 @@ void i2c_hid_core_remove(struct i2c_client *client)
        struct i2c_hid *ihid = i2c_get_clientdata(client);
        struct hid_device *hid;
 
+       i2c_hid_core_power_down(ihid);
+
        hid = ihid->hid;
        hid_destroy_device(hid);
 
@@ -1069,8 +1101,6 @@ void i2c_hid_core_remove(struct i2c_client *client)
 
        if (ihid->bufsize)
                i2c_hid_free_buffers(ihid);
-
-       i2c_hid_core_power_down(ihid);
 }
 EXPORT_SYMBOL_GPL(i2c_hid_core_remove);