HID: i2c-hid: ensure various commands do not interfere with each other

i2c-hid uses 2 shared buffers: command and "raw" input buffer for
sending requests to peripherals and read data from peripherals when
executing variety of commands. Such commands include reading of HID
registers, requesting particular power mode, getting and setting
reports and so on. Because all such requests use the same 2 buffers
they should not execute simultaneously.

Fix this by introducing "cmd_lock" mutex and acquire it whenever
we needs to access ihid->cmdbuf or idid->rawbuf.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 632eaf9e11a6b679b0c1733167d8b00940a5d657..2f8a9d3f1e861e6b3988ef790e5a7f9521e7c733 100644 (file)
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -105,6 +105,7 @@ struct i2c_hid {
 
        wait_queue_head_t       wait;           /* For waiting the interrupt */
 
+       struct mutex            cmd_lock;       /* protects cmdbuf and rawbuf */
        struct mutex            reset_lock;
 
        struct i2chid_ops       *ops;
@@ -220,6 +221,8 @@ static int i2c_hid_xfer(struct i2c_hid *ihid,
 static int i2c_hid_read_register(struct i2c_hid *ihid, __le16 reg,
                                 void *buf, size_t len)
 {
+       guard(mutex)(&ihid->cmd_lock);
+
        *(__le16 *)ihid->cmdbuf = reg;
 
        return i2c_hid_xfer(ihid, ihid->cmdbuf, sizeof(__le16), buf, len);
@@ -252,6 +255,8 @@ static int i2c_hid_get_report(struct i2c_hid *ihid,
 
        i2c_hid_dbg(ihid, "%s\n", __func__);
 
+       guard(mutex)(&ihid->cmd_lock);
+
        /* Command register goes first */
        *(__le16 *)ihid->cmdbuf = ihid->hdesc.wCommandRegister;
        length += sizeof(__le16);
@@ -342,6 +347,8 @@ static int i2c_hid_set_or_send_report(struct i2c_hid *ihid,
        if (!do_set && le16_to_cpu(ihid->hdesc.wMaxOutputLength) == 0)
                return -ENOSYS;
 
+       guard(mutex)(&ihid->cmd_lock);
+
        if (do_set) {
                /* Command register goes first */
                *(__le16 *)ihid->cmdbuf = ihid->hdesc.wCommandRegister;
@@ -384,6 +391,8 @@ static int i2c_hid_set_power_command(struct i2c_hid *ihid, int power_state)
 {
        size_t length;
 
+       guard(mutex)(&ihid->cmd_lock);
+
        /* SET_POWER uses command register */
        *(__le16 *)ihid->cmdbuf = ihid->hdesc.wCommandRegister;
        length = sizeof(__le16);
@@ -440,25 +449,27 @@ static int i2c_hid_start_hwreset(struct i2c_hid *ihid)
        if (ret)
                return ret;
 
-       /* Prepare reset command. Command register goes first. */
-       *(__le16 *)ihid->cmdbuf = ihid->hdesc.wCommandRegister;
-       length += sizeof(__le16);
-       /* Next is RESET command itself */
-       length += i2c_hid_encode_command(ihid->cmdbuf + length,
-                                        I2C_HID_OPCODE_RESET, 0, 0);
+       scoped_guard(mutex, &ihid->cmd_lock) {
+               /* Prepare reset command. Command register goes first. */
+               *(__le16 *)ihid->cmdbuf = ihid->hdesc.wCommandRegister;
+               length += sizeof(__le16);
+               /* Next is RESET command itself */
+               length += i2c_hid_encode_command(ihid->cmdbuf + length,
+                                                I2C_HID_OPCODE_RESET, 0, 0);
 
-       set_bit(I2C_HID_RESET_PENDING, &ihid->flags);
+               set_bit(I2C_HID_RESET_PENDING, &ihid->flags);
 
-       ret = i2c_hid_xfer(ihid, ihid->cmdbuf, length, NULL, 0);
-       if (ret) {
-               dev_err(&ihid->client->dev,
-                       "failed to reset device: %d\n", ret);
-               goto err_clear_reset;
-       }
+               ret = i2c_hid_xfer(ihid, ihid->cmdbuf, length, NULL, 0);
+               if (ret) {
+                       dev_err(&ihid->client->dev,
+                               "failed to reset device: %d\n", ret);
+                       break;
+               }
 
-       return 0;
+               return 0;
+       }
 
-err_clear_reset:
+       /* Clean up if sending reset command failed */
        clear_bit(I2C_HID_RESET_PENDING, &ihid->flags);
        i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP);
        return ret;
@@ -1200,6 +1211,7 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
        ihid->is_panel_follower = drm_is_panel_follower(&client->dev);
 
        init_waitqueue_head(&ihid->wait);
+       mutex_init(&ihid->cmd_lock);
        mutex_init(&ihid->reset_lock);
        INIT_WORK(&ihid->panel_follower_prepare_work, ihid_core_panel_prepare_work);