HID: thingm: improve locking

Reading from the device consists of two operations: sending the read
command and the actual read from the device. If the device is accessed
in between we might read wrong data. Therefore protect the full sequence
of both operations with a mutex.
Also change the semantics of thingm_recv to include both operations.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
This commit is contained in:
Heiner Kallweit 2016-03-01 20:49:54 +01:00 committed by Jiri Kosina
parent c46fab2870
commit 43a4a04d4d

View File

@ -77,9 +77,13 @@ static int thingm_send(struct thingm_device *tdev, u8 buf[REPORT_SIZE])
buf[0], buf[1], buf[2], buf[3], buf[4], buf[0], buf[1], buf[2], buf[3], buf[4],
buf[5], buf[6], buf[7], buf[8]); buf[5], buf[6], buf[7], buf[8]);
mutex_lock(&tdev->lock);
ret = hid_hw_raw_request(tdev->hdev, buf[0], buf, REPORT_SIZE, ret = hid_hw_raw_request(tdev->hdev, buf[0], buf, REPORT_SIZE,
HID_FEATURE_REPORT, HID_REQ_SET_REPORT); HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
mutex_unlock(&tdev->lock);
return ret < 0 ? ret : 0; return ret < 0 ? ret : 0;
} }
@ -87,16 +91,31 @@ static int thingm_recv(struct thingm_device *tdev, u8 buf[REPORT_SIZE])
{ {
int ret; int ret;
/*
* A read consists of two operations: sending the read command
* and the actual read from the device. Use the mutex to protect
* the full sequence of both operations.
*/
mutex_lock(&tdev->lock);
ret = hid_hw_raw_request(tdev->hdev, buf[0], buf, REPORT_SIZE,
HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
if (ret < 0)
goto err;
ret = hid_hw_raw_request(tdev->hdev, buf[0], buf, REPORT_SIZE, ret = hid_hw_raw_request(tdev->hdev, buf[0], buf, REPORT_SIZE,
HID_FEATURE_REPORT, HID_REQ_GET_REPORT); HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
if (ret < 0) if (ret < 0)
return ret; goto err;
ret = 0;
hid_dbg(tdev->hdev, "<- %d %c %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx\n", hid_dbg(tdev->hdev, "<- %d %c %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx\n",
buf[0], buf[1], buf[2], buf[3], buf[4], buf[0], buf[1], buf[2], buf[3], buf[4],
buf[5], buf[6], buf[7], buf[8]); buf[5], buf[6], buf[7], buf[8]);
err:
return 0; mutex_unlock(&tdev->lock);
return ret;
} }
static int thingm_version(struct thingm_device *tdev) static int thingm_version(struct thingm_device *tdev)
@ -104,10 +123,6 @@ static int thingm_version(struct thingm_device *tdev)
u8 buf[REPORT_SIZE] = { REPORT_ID, 'v', 0, 0, 0, 0, 0, 0, 0 }; u8 buf[REPORT_SIZE] = { REPORT_ID, 'v', 0, 0, 0, 0, 0, 0, 0 };
int err; int err;
err = thingm_send(tdev, buf);
if (err)
return err;
err = thingm_recv(tdev, buf); err = thingm_recv(tdev, buf);
if (err) if (err)
return err; return err;
@ -135,14 +150,10 @@ static int thingm_led_set(struct led_classdev *ldev,
struct thingm_led *led = container_of(ldev, struct thingm_led, ldev); struct thingm_led *led = container_of(ldev, struct thingm_led, ldev);
int ret; int ret;
mutex_lock(&led->rgb->tdev->lock);
ret = thingm_write_color(led->rgb); ret = thingm_write_color(led->rgb);
if (ret) if (ret)
hid_err(led->rgb->tdev->hdev, "failed to write color\n"); hid_err(led->rgb->tdev->hdev, "failed to write color\n");
mutex_unlock(&led->rgb->tdev->lock);
return ret; return ret;
} }