Skip to content

Commit

Permalink
HID: wiimote: Simplify synchronization
Browse files Browse the repository at this point in the history
The new locking scheme in HID core allows us to remove a bit of synchronization.
Since the HID layer acts synchronously we simply register input core last and
there are no synchonization issues anymore.
Also register sysfs files after that to simplify the code.

Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
  • Loading branch information
David Herrmann authored and Jiri Kosina committed Aug 23, 2011
1 parent f5fc879 commit 3989ef6
Showing 1 changed file with 27 additions and 51 deletions.
78 changes: 27 additions & 51 deletions drivers/hid/hid-wiimote.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
* any later version.
*/

#include <linux/atomic.h>
#include <linux/device.h>
#include <linux/hid.h>
#include <linux/input.h>
Expand All @@ -33,7 +32,6 @@ struct wiimote_state {
};

struct wiimote_data {
atomic_t ready;
struct hid_device *hdev;
struct input_dev *input;

Expand Down Expand Up @@ -200,9 +198,6 @@ static ssize_t wiifs_led_show_##num(struct device *dev, \
unsigned long flags; \
int state; \
\
if (!atomic_read(&wdata->ready)) \
return -EBUSY; \
\
spin_lock_irqsave(&wdata->state.lock, flags); \
state = !!(wdata->state.flags & WIIPROTO_FLAG_LED##num); \
spin_unlock_irqrestore(&wdata->state.lock, flags); \
Expand All @@ -217,9 +212,6 @@ static ssize_t wiifs_led_set_##num(struct device *dev, \
unsigned long flags; \
__u8 state; \
\
if (!atomic_read(&wdata->ready)) \
return -EBUSY; \
\
spin_lock_irqsave(&wdata->state.lock, flags); \
\
state = wdata->state.flags; \
Expand All @@ -244,13 +236,6 @@ wiifs_led_show_set(4);
static int wiimote_input_event(struct input_dev *dev, unsigned int type,
unsigned int code, int value)
{
struct wiimote_data *wdata = input_get_drvdata(dev);

if (!atomic_read(&wdata->ready))
return -EBUSY;
/* smp_rmb: Make sure wdata->xy is available when wdata->ready is 1 */
smp_rmb();

return 0;
}

Expand Down Expand Up @@ -300,11 +285,6 @@ static int wiimote_hid_event(struct hid_device *hdev, struct hid_report *report,
int i;
unsigned long flags;

if (!atomic_read(&wdata->ready))
return -EBUSY;
/* smp_rmb: Make sure wdata->xy is available when wdata->ready is 1 */
smp_rmb();

if (size < 1)
return -EINVAL;

Expand Down Expand Up @@ -362,6 +342,15 @@ static struct wiimote_data *wiimote_create(struct hid_device *hdev)

static void wiimote_destroy(struct wiimote_data *wdata)
{
device_remove_file(&wdata->hdev->dev, &dev_attr_led1);
device_remove_file(&wdata->hdev->dev, &dev_attr_led2);
device_remove_file(&wdata->hdev->dev, &dev_attr_led3);
device_remove_file(&wdata->hdev->dev, &dev_attr_led4);

input_unregister_device(wdata->input);
cancel_work_sync(&wdata->worker);
hid_hw_stop(wdata->hdev);

kfree(wdata);
}

Expand All @@ -377,19 +366,6 @@ static int wiimote_hid_probe(struct hid_device *hdev,
return -ENOMEM;
}

ret = device_create_file(&hdev->dev, &dev_attr_led1);
if (ret)
goto err;
ret = device_create_file(&hdev->dev, &dev_attr_led2);
if (ret)
goto err;
ret = device_create_file(&hdev->dev, &dev_attr_led3);
if (ret)
goto err;
ret = device_create_file(&hdev->dev, &dev_attr_led4);
if (ret)
goto err;

ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "HID parse failed\n");
Expand All @@ -408,9 +384,19 @@ static int wiimote_hid_probe(struct hid_device *hdev,
goto err_stop;
}

/* smp_wmb: Write wdata->xy first before wdata->ready is set to 1 */
smp_wmb();
atomic_set(&wdata->ready, 1);
ret = device_create_file(&hdev->dev, &dev_attr_led1);
if (ret)
goto err_free;
ret = device_create_file(&hdev->dev, &dev_attr_led2);
if (ret)
goto err_free;
ret = device_create_file(&hdev->dev, &dev_attr_led3);
if (ret)
goto err_free;
ret = device_create_file(&hdev->dev, &dev_attr_led4);
if (ret)
goto err_free;

hid_info(hdev, "New device registered\n");

/* by default set led1 after device initialization */
Expand All @@ -420,15 +406,15 @@ static int wiimote_hid_probe(struct hid_device *hdev,

return 0;

err_free:
wiimote_destroy(wdata);
return ret;

err_stop:
hid_hw_stop(hdev);
err:
input_free_device(wdata->input);
device_remove_file(&hdev->dev, &dev_attr_led1);
device_remove_file(&hdev->dev, &dev_attr_led2);
device_remove_file(&hdev->dev, &dev_attr_led3);
device_remove_file(&hdev->dev, &dev_attr_led4);
wiimote_destroy(wdata);
kfree(wdata);
return ret;
}

Expand All @@ -437,16 +423,6 @@ static void wiimote_hid_remove(struct hid_device *hdev)
struct wiimote_data *wdata = hid_get_drvdata(hdev);

hid_info(hdev, "Device removed\n");

device_remove_file(&hdev->dev, &dev_attr_led1);
device_remove_file(&hdev->dev, &dev_attr_led2);
device_remove_file(&hdev->dev, &dev_attr_led3);
device_remove_file(&hdev->dev, &dev_attr_led4);

hid_hw_stop(hdev);
input_unregister_device(wdata->input);

cancel_work_sync(&wdata->worker);
wiimote_destroy(wdata);
}

Expand Down

0 comments on commit 3989ef6

Please sign in to comment.