Input: elan_i2c - switch to using cleanup functions

Start using __free() and guard() primitives to simplify the code
and error handling. This makes the code more compact and error
handling more robust by ensuring that locks are released in all
code paths when control leaves critical section and all allocated
memory is freed.

Link: https://lore.kernel.org/r/ZsrBC7qDbOvAaI-W@google.com
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index ce96513b34f64f2865abcc5549b4aaac40f55d7f..ef44bc027c30677fe192b2e599a831949fa675de 100644 (file)
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -541,7 +541,8 @@ static int elan_update_firmware(struct elan_tp_data *data,
 
        dev_dbg(&client->dev, "Starting firmware update....\n");
 
-       disable_irq(client->irq);
+       guard(disable_irq)(&client->irq);
+
        data->in_fw_update = true;
 
        retval = __elan_update_firmware(data, fw);
@@ -555,7 +556,6 @@ static int elan_update_firmware(struct elan_tp_data *data,
        }
 
        data->in_fw_update = false;
-       enable_irq(client->irq);
 
        return retval;
 }
@@ -621,8 +621,6 @@ static ssize_t elan_sysfs_update_fw(struct device *dev,
                                    const char *buf, size_t count)
 {
        struct elan_tp_data *data = dev_get_drvdata(dev);
-       const struct firmware *fw;
-       char *fw_name;
        int error;
        const u8 *fw_signature;
        static const u8 signature[] = {0xAA, 0x55, 0xCC, 0x33, 0xFF, 0xFF};
@@ -631,15 +629,16 @@ static ssize_t elan_sysfs_update_fw(struct device *dev,
                return -EINVAL;
 
        /* Look for a firmware with the product id appended. */
-       fw_name = kasprintf(GFP_KERNEL, ETP_FW_NAME, data->product_id);
+       const char *fw_name __free(kfree) =
+               kasprintf(GFP_KERNEL, ETP_FW_NAME, data->product_id);
        if (!fw_name) {
                dev_err(dev, "failed to allocate memory for firmware name\n");
                return -ENOMEM;
        }
 
        dev_info(dev, "requesting fw '%s'\n", fw_name);
+       const struct firmware *fw __free(firmware) = NULL;
        error = request_firmware(&fw, fw_name, dev);
-       kfree(fw_name);
        if (error) {
                dev_err(dev, "failed to request firmware: %d\n", error);
                return error;
@@ -651,46 +650,36 @@ static ssize_t elan_sysfs_update_fw(struct device *dev,
                dev_err(dev, "signature mismatch (expected %*ph, got %*ph)\n",
                        (int)sizeof(signature), signature,
                        (int)sizeof(signature), fw_signature);
-               error = -EBADF;
-               goto out_release_fw;
+               return -EBADF;
        }
 
-       error = mutex_lock_interruptible(&data->sysfs_mutex);
-       if (error)
-               goto out_release_fw;
-
-       error = elan_update_firmware(data, fw);
-
-       mutex_unlock(&data->sysfs_mutex);
+       scoped_cond_guard(mutex_intr, return -EINTR, &data->sysfs_mutex) {
+               error = elan_update_firmware(data, fw);
+               if (error)
+                       return error;
+       }
 
-out_release_fw:
-       release_firmware(fw);
-       return error ?: count;
+       return count;
 }
 
-static ssize_t calibrate_store(struct device *dev,
-                              struct device_attribute *attr,
-                              const char *buf, size_t count)
+static int elan_calibrate(struct elan_tp_data *data)
 {
-       struct i2c_client *client = to_i2c_client(dev);
-       struct elan_tp_data *data = i2c_get_clientdata(client);
+       struct i2c_client *client = data->client;
+       struct device *dev = &client->dev;
        int tries = 20;
        int retval;
        int error;
        u8 val[ETP_CALIBRATE_MAX_LEN];
 
-       retval = mutex_lock_interruptible(&data->sysfs_mutex);
-       if (retval)
-               return retval;
-
-       disable_irq(client->irq);
+       guard(disable_irq)(&client->irq);
 
        data->mode |= ETP_ENABLE_CALIBRATE;
        retval = data->ops->set_mode(client, data->mode);
        if (retval) {
+               data->mode &= ~ETP_ENABLE_CALIBRATE;
                dev_err(dev, "failed to enable calibration mode: %d\n",
                        retval);
-               goto out;
+               return retval;
        }
 
        retval = data->ops->calibrate(client);
@@ -728,10 +717,24 @@ out_disable_calibrate:
                if (!retval)
                        retval = error;
        }
-out:
-       enable_irq(client->irq);
-       mutex_unlock(&data->sysfs_mutex);
-       return retval ?: count;
+       return retval;
+}
+
+static ssize_t calibrate_store(struct device *dev,
+                              struct device_attribute *attr,
+                              const char *buf, size_t count)
+{
+       struct i2c_client *client = to_i2c_client(dev);
+       struct elan_tp_data *data = i2c_get_clientdata(client);
+       int error;
+
+       scoped_cond_guard(mutex_intr, return -EINTR, &data->sysfs_mutex) {
+               error = elan_calibrate(data);
+               if (error)
+                       return error;
+       }
+
+       return count;
 }
 
 static ssize_t elan_sysfs_read_mode(struct device *dev,
@@ -743,16 +746,11 @@ static ssize_t elan_sysfs_read_mode(struct device *dev,
        int error;
        enum tp_mode mode;
 
-       error = mutex_lock_interruptible(&data->sysfs_mutex);
-       if (error)
-               return error;
-
-       error = data->ops->iap_get_mode(data->client, &mode);
-
-       mutex_unlock(&data->sysfs_mutex);
-
-       if (error)
-               return error;
+       scoped_cond_guard(mutex_intr, return -EINTR, &data->sysfs_mutex) {
+               error = data->ops->iap_get_mode(data->client, &mode);
+               if (error)
+                       return error;
+       }
 
        return sysfs_emit(buf, "%d\n", (int)mode);
 }
@@ -783,44 +781,40 @@ static const struct attribute_group elan_sysfs_group = {
        .attrs = elan_sysfs_entries,
 };
 
-static ssize_t acquire_store(struct device *dev, struct device_attribute *attr,
-                            const char *buf, size_t count)
+static int elan_acquire_baseline(struct elan_tp_data *data)
 {
-       struct i2c_client *client = to_i2c_client(dev);
-       struct elan_tp_data *data = i2c_get_clientdata(client);
-       int error;
+       struct i2c_client *client = data->client;
+       struct device *dev = &client->dev;
        int retval;
+       int error;
 
-       retval = mutex_lock_interruptible(&data->sysfs_mutex);
-       if (retval)
-               return retval;
-
-       disable_irq(client->irq);
+       guard(disable_irq)(&client->irq);
 
        data->baseline_ready = false;
 
        data->mode |= ETP_ENABLE_CALIBRATE;
-       retval = data->ops->set_mode(data->client, data->mode);
+       retval = data->ops->set_mode(client, data->mode);
        if (retval) {
+               data->mode &= ~ETP_ENABLE_CALIBRATE;
                dev_err(dev, "Failed to enable calibration mode to get baseline: %d\n",
                        retval);
-               goto out;
+               return retval;
        }
 
        msleep(250);
 
-       retval = data->ops->get_baseline_data(data->client, true,
+       retval = data->ops->get_baseline_data(client, true,
                                              &data->max_baseline);
        if (retval) {
-               dev_err(dev, "Failed to read max baseline form device: %d\n",
+               dev_err(dev, "Failed to read max baseline from device: %d\n",
                        retval);
                goto out_disable_calibrate;
        }
 
-       retval = data->ops->get_baseline_data(data->client, false,
+       retval = data->ops->get_baseline_data(client, false,
                                              &data->min_baseline);
        if (retval) {
-               dev_err(dev, "Failed to read min baseline form device: %d\n",
+               dev_err(dev, "Failed to read min baseline from device: %d\n",
                        retval);
                goto out_disable_calibrate;
        }
@@ -829,17 +823,31 @@ static ssize_t acquire_store(struct device *dev, struct device_attribute *attr,
 
 out_disable_calibrate:
        data->mode &= ~ETP_ENABLE_CALIBRATE;
-       error = data->ops->set_mode(data->client, data->mode);
+       error = data->ops->set_mode(client, data->mode);
        if (error) {
                dev_err(dev, "Failed to disable calibration mode after acquiring baseline: %d\n",
                        error);
                if (!retval)
                        retval = error;
        }
-out:
-       enable_irq(client->irq);
-       mutex_unlock(&data->sysfs_mutex);
-       return retval ?: count;
+
+       return retval;
+}
+
+static ssize_t acquire_store(struct device *dev, struct device_attribute *attr,
+                            const char *buf, size_t count)
+{
+       struct i2c_client *client = to_i2c_client(dev);
+       struct elan_tp_data *data = i2c_get_clientdata(client);
+       int error;
+
+       scoped_cond_guard(mutex_intr, return -EINTR, &data->sysfs_mutex) {
+               error = elan_acquire_baseline(data);
+               if (error)
+                       return error;
+       }
+
+       return count;
 }
 
 static ssize_t min_show(struct device *dev,
@@ -847,22 +855,15 @@ static ssize_t min_show(struct device *dev,
 {
        struct i2c_client *client = to_i2c_client(dev);
        struct elan_tp_data *data = i2c_get_clientdata(client);
-       int retval;
 
-       retval = mutex_lock_interruptible(&data->sysfs_mutex);
-       if (retval)
-               return retval;
+       scoped_guard(mutex_intr, &data->sysfs_mutex) {
+               if (!data->baseline_ready)
+                       return -ENODATA;
 
-       if (!data->baseline_ready) {
-               retval = -ENODATA;
-               goto out;
+               return sysfs_emit(buf, "%d", data->min_baseline);
        }
 
-       retval = sysfs_emit(buf, "%d", data->min_baseline);
-
-out:
-       mutex_unlock(&data->sysfs_mutex);
-       return retval;
+       return -EINTR;
 }
 
 static ssize_t max_show(struct device *dev,
@@ -870,25 +871,17 @@ static ssize_t max_show(struct device *dev,
 {
        struct i2c_client *client = to_i2c_client(dev);
        struct elan_tp_data *data = i2c_get_clientdata(client);
-       int retval;
 
-       retval = mutex_lock_interruptible(&data->sysfs_mutex);
-       if (retval)
-               return retval;
+       scoped_guard(mutex_intr, &data->sysfs_mutex) {
+               if (!data->baseline_ready)
+                       return -ENODATA;
 
-       if (!data->baseline_ready) {
-               retval = -ENODATA;
-               goto out;
+               return sysfs_emit(buf, "%d", data->max_baseline);
        }
 
-       retval = sysfs_emit(buf, "%d", data->max_baseline);
-
-out:
-       mutex_unlock(&data->sysfs_mutex);
-       return retval;
+       return -EINTR;
 }
 
-
 static DEVICE_ATTR_WO(acquire);
 static DEVICE_ATTR_RO(min);
 static DEVICE_ATTR_RO(max);
@@ -1323,43 +1316,54 @@ static int elan_probe(struct i2c_client *client)
        return 0;
 }
 
+static int __elan_suspend(struct elan_tp_data *data)
+{
+       struct i2c_client *client = data->client;
+       int error;
+
+       if (device_may_wakeup(&client->dev))
+               return elan_sleep(data);
+
+       /* Touchpad is not a wakeup source */
+       error = elan_set_power(data, false);
+       if (error)
+               return error;
+
+       error = regulator_disable(data->vcc);
+       if (error) {
+               dev_err(&client->dev,
+                       "failed to disable regulator when suspending: %d\n",
+                       error);
+               /* Attempt to power the chip back up */
+               elan_set_power(data, true);
+               return error;
+       }
+
+       return 0;
+}
+
 static int elan_suspend(struct device *dev)
 {
        struct i2c_client *client = to_i2c_client(dev);
        struct elan_tp_data *data = i2c_get_clientdata(client);
-       int ret;
+       int error;
 
        /*
         * We are taking the mutex to make sure sysfs operations are
         * complete before we attempt to bring the device into low[er]
         * power mode.
         */
-       ret = mutex_lock_interruptible(&data->sysfs_mutex);
-       if (ret)
-               return ret;
-
-       disable_irq(client->irq);
+       scoped_cond_guard(mutex_intr, return -EINTR, &data->sysfs_mutex) {
+               disable_irq(client->irq);
 
-       if (device_may_wakeup(dev)) {
-               ret = elan_sleep(data);
-       } else {
-               ret = elan_set_power(data, false);
-               if (ret)
-                       goto err;
-
-               ret = regulator_disable(data->vcc);
-               if (ret) {
-                       dev_err(dev, "error %d disabling regulator\n", ret);
-                       /* Attempt to power the chip back up */
-                       elan_set_power(data, true);
+               error = __elan_suspend(data);
+               if (error) {
+                       enable_irq(client->irq);
+                       return error;
                }
        }
 
-err:
-       if (ret)
-               enable_irq(client->irq);
-       mutex_unlock(&data->sysfs_mutex);
-       return ret;
+       return 0;
 }
 
 static int elan_resume(struct device *dev)

diff --git a/drivers/input/mouse/elan_i2c_i2c.c b/drivers/input/mouse/elan_i2c_i2c.c
index 13dc097eb6c652983399c1fbefea7475a4ea0ff2..5ed13c8d976e18f8affb8eec944fc25dd68facf9 100644 (file)
--- a/drivers/input/mouse/elan_i2c_i2c.c
+++ b/drivers/input/mouse/elan_i2c_i2c.c
@@ -628,12 +628,11 @@ static int elan_i2c_write_fw_block(struct i2c_client *client, u16 fw_page_size,
                                   const u8 *page, u16 checksum, int idx)
 {
        struct device *dev = &client->dev;
-       u8 *page_store;
        u8 val[3];
        u16 result;
        int ret, error;
 
-       page_store = kmalloc(fw_page_size + 4, GFP_KERNEL);
+       u8 *page_store __free(kfree) = kmalloc(fw_page_size + 4, GFP_KERNEL);
        if (!page_store)
                return -ENOMEM;
 
@@ -647,7 +646,7 @@ static int elan_i2c_write_fw_block(struct i2c_client *client, u16 fw_page_size,
        if (ret != fw_page_size + 4) {
                error = ret < 0 ? ret : -EIO;
                dev_err(dev, "Failed to write page %d: %d\n", idx, error);
-               goto exit;
+               return error;
        }
 
        /* Wait for F/W to update one page ROM data. */
@@ -656,20 +655,17 @@ static int elan_i2c_write_fw_block(struct i2c_client *client, u16 fw_page_size,
        error = elan_i2c_read_cmd(client, ETP_I2C_IAP_CTRL_CMD, val);
        if (error) {
                dev_err(dev, "Failed to read IAP write result: %d\n", error);
-               goto exit;
+               return error;
        }
 
        result = le16_to_cpup((__le16 *)val);
        if (result & (ETP_FW_IAP_PAGE_ERR | ETP_FW_IAP_INTF_ERR)) {
                dev_err(dev, "IAP reports failed write: %04hx\n",
                        result);
-               error = -EIO;
-               goto exit;
+               return -EIO;
        }
 
-exit:
-       kfree(page_store);
-       return error;
+       return 0;
 }
 
 static int elan_i2c_finish_fw_update(struct i2c_client *client,