Re: [RFC 05/12] HID: wiimote: Synchronize wiimote input and hid event handling
flat view
From: Oliver Neukum <oliver@neukum.org>
Date: 2011-06-15 06:42:18
Am Mittwoch, 15. Juni 2011, 01:45:50 schrieb David Herrmann:
The wiimote first starts HID hardware and then registers the input device. We need to synchronize the startup so no event handler will start parsing events when the wiimote device is not ready, yet.
Hi, here the fun begins. Using atomic ops is not for the faint of heart. The problem is that atomic_t guarantees atomicity, not ordering. Tasks on on other CPUs reading wdata->ready==1 does not mean that they'll read anything else written before that as it was written or the old value. You need smp barriers.
quoted hunk ↗ jump to hunk
Signed-off-by: David Herrmann <redacted> --- drivers/hid/hid-wiimote.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)diff --git a/drivers/hid/hid-wiimote.c b/drivers/hid/hid-wiimote.c index 9fa4fe1..4dbb2c1 100644 --- a/drivers/hid/hid-wiimote.c +++ b/drivers/hid/hid-wiimote.c@@ -10,6 +10,7 @@ * any later version. */ +#include <linux/atomic.h> #include <linux/device.h> #include <linux/hid.h> #include <linux/input.h>@@ -20,6 +21,7 @@ #define WIIMOTE_NAME "Nintendo Wii Remote" struct wiimote_data { + atomic_t ready; struct hid_device *hdev; struct input_dev *input; };@@ -27,12 +29,22 @@ struct wiimote_data { 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; +
Here an smp_rmb()
quoted hunk ↗ jump to hunk
return 0; } static int wiimote_hid_event(struct hid_device *hdev, struct hid_report *report, u8 *raw_data, int size) { + struct wiimote_data *wdata = hid_get_drvdata(hdev); + + if (!atomic_read(&wdata->ready)) + return -EBUSY; + if (size < 1) return -EINVAL;@@ -98,6 +110,7 @@ static int wiimote_hid_probe(struct hid_device *hdev, goto err_stop; }
Here an smp_wmb() It is redundant because input_register_device takes a spinlock which implies a barrier, but you'll make the automated checking tools happier.
quoted hunk ↗ jump to hunk
+ atomic_set(&wdata->ready, 1); hid_info(hdev, "New device registered\n"); return 0;
Regards Oliver