diff mbox

[06/16] HID: wiimote: Enable accelerometer on request

Message ID 1311869316-17128-7-git-send-email-dh.herrmann@googlemail.com (mailing list archive)
State New, archived
Delegated to: Jiri Kosina
Headers show

Commit Message

David Herrmann July 28, 2011, 4:08 p.m. UTC
The wiimote has an internal accelerometer which can report data to the host.
Userspace may now write to a new sysfs file to make the driver enable
accelerometer reporting. This is not enabled by default to reduce power
consumption of the wiimote. Accelerometer data is reported every few
milliseconds and thus consumes much bluetooth bandwidth which costs much energy
of the wiimote.
By writing 0 to the sysfs file, accelerometer reporting is disabled again.

The wiimotes accelerometer does not need to be enabled explicitely, we only need
to set the DRM to a mode which includes accelerometer data.

Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
 Documentation/ABI/testing/sysfs-driver-hid-wiimote |    8 +++
 drivers/hid/hid-wiimote.c                          |   57 ++++++++++++++++++++
 2 files changed, 65 insertions(+), 0 deletions(-)

Comments

Oliver Neukum July 28, 2011, 6:08 p.m. UTC | #1
Am Donnerstag, 28. Juli 2011, 18:08:26 schrieb David Herrmann:
> The wiimote has an internal accelerometer which can report data to the host.
> Userspace may now write to a new sysfs file to make the driver enable
> accelerometer reporting. This is not enabled by default to reduce power
> consumption of the wiimote. Accelerometer data is reported every few
> milliseconds and thus consumes much bluetooth bandwidth which costs much energy
> of the wiimote.
> By writing 0 to the sysfs file, accelerometer reporting is disabled again.

This is an odd interface. Why can't you request the accelerometer data when
a device file dedicated to the accelerometer is opened?

	Regards
		Oliver
David Herrmann July 28, 2011, 7:01 p.m. UTC | #2
On Thu, Jul 28, 2011 at 8:08 PM, Oliver Neukum <oneukum@suse.de> wrote:
> Am Donnerstag, 28. Juli 2011, 18:08:26 schrieb David Herrmann:
>> The wiimote has an internal accelerometer which can report data to the host.
>> Userspace may now write to a new sysfs file to make the driver enable
>> accelerometer reporting. This is not enabled by default to reduce power
>> consumption of the wiimote. Accelerometer data is reported every few
>> milliseconds and thus consumes much bluetooth bandwidth which costs much energy
>> of the wiimote.
>> By writing 0 to the sysfs file, accelerometer reporting is disabled again.
>
> This is an odd interface. Why can't you request the accelerometer data when
> a device file dedicated to the accelerometer is opened?

The API consists of sysfs and an input device. There is no way to be
able to see when accelerometer data is requested. Of course, I could
use ->open() callback on the input-layer. However, many use cases do
not require accelerometer data and bluetooth traffic is increased
heavily when accelerometer reports are enabled.
For instance if you only need button-reports, then one package per
button press is transmitted. With accelerometer reports, every 5ms a
package is transmitted. And the battery of the wiimote is quite
small...

I could create multiple input interfaces, one for button data, one for
accelerometer, one for IR etc. however, how can userspace know which
one is for which interface BEFORE opening it? It could open all of
them and close the ones it doesn't need, but is that better than
having a sysfs attribute?

I can see the problem with my approach. If multiple applications use
the wiimote and one disables the accelerometer, then the other
application has it also disabled. However, what other API would be
more appropriate here?

A char-dev would make the interface very easy but I tried to avoid
cdevs and ioctl()s since I was told so.

>        Regards
>                Oliver

Regards
David
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Oliver Neukum July 28, 2011, 7:32 p.m. UTC | #3
Am Donnerstag, 28. Juli 2011, 21:01:34 schrieb David Herrmann:
> I can see the problem with my approach. If multiple applications use
> the wiimote and one disables the accelerometer, then the other
> application has it also disabled. However, what other API would be
> more appropriate here?

So arguments can be seen for both. I suggest you ask this question
specifically on the lists before you introduce an API here.

	Regards
		Oliver
diff mbox

Patch

diff --git a/Documentation/ABI/testing/sysfs-driver-hid-wiimote b/Documentation/ABI/testing/sysfs-driver-hid-wiimote
index 235dd47..d8ccea8 100644
--- a/Documentation/ABI/testing/sysfs-driver-hid-wiimote
+++ b/Documentation/ABI/testing/sysfs-driver-hid-wiimote
@@ -16,3 +16,11 @@  Contact:	David Herrmann <dh.herrmann@googlemail.com>
 Description:	Writing 1 to this file enables the rumble motor on the wiimote
 		and writing 0 disables it again. Reading from this file returns
 		1 if rumble is on and 0 if it is off.
+
+What:		/sys/bus/hid/drivers/wiimote/<dev>/accelerometer
+Date:		July 2011
+KernelVersion:	3.2
+Contact:	David Herrmann <dh.herrmann@googlemail.com>
+Description:	Writing 1 to this file enables accelerometer data reporting of
+		the wiimote, 0 disables it. Reading from this file returns the
+		current value.
diff --git a/drivers/hid/hid-wiimote.c b/drivers/hid/hid-wiimote.c
index bac4410..13c27b7 100644
--- a/drivers/hid/hid-wiimote.c
+++ b/drivers/hid/hid-wiimote.c
@@ -51,6 +51,7 @@  struct wiimote_data {
 #define WIIPROTO_FLAG_LED3		0x04
 #define WIIPROTO_FLAG_LED4		0x08
 #define WIIPROTO_FLAG_RUMBLE		0x10
+#define WIIPROTO_FLAG_ACCEL		0x20
 #define WIIPROTO_FLAGS_LEDS (WIIPROTO_FLAG_LED1 | WIIPROTO_FLAG_LED2 | \
 					WIIPROTO_FLAG_LED3 | WIIPROTO_FLAG_LED4)
 
@@ -257,6 +258,20 @@  static void wiiproto_req_drm(struct wiimote_data *wdata, __u8 drm)
 	wiimote_queue(wdata, cmd, sizeof(cmd));
 }
 
+static void wiiproto_req_accel(struct wiimote_data *wdata, __u8 accel)
+{
+	accel = !!accel;
+	if (accel == !!(wdata->state.flags & WIIPROTO_FLAG_ACCEL))
+		return;
+
+	if (accel)
+		wdata->state.flags |= WIIPROTO_FLAG_ACCEL;
+	else
+		wdata->state.flags &= ~WIIPROTO_FLAG_ACCEL;
+
+	wiiproto_req_drm(wdata, WIIPROTO_REQ_NULL);
+}
+
 #define wiifs_led_show_set(num)						\
 static ssize_t wiifs_led_show_##num(struct device *dev,			\
 			struct device_attribute *attr, char *buf)	\
@@ -343,6 +358,43 @@  static ssize_t wiifs_rumble_set(struct device *dev,
 static DEVICE_ATTR(rumble, S_IRUGO | S_IWUSR, wiifs_rumble_show,
 							wiifs_rumble_set);
 
+static ssize_t wiifs_accel_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct wiimote_data *wdata = dev_to_wii(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_ACCEL);
+	spin_unlock_irqrestore(&wdata->state.lock, flags);
+
+	return sprintf(buf, "%d\n", state);
+}
+
+static ssize_t wiifs_accel_set(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct wiimote_data *wdata = dev_to_wii(dev);
+	int tmp = simple_strtoul(buf, NULL, 10);
+	unsigned long flags;
+
+	if (!atomic_read(&wdata->ready))
+		return -EBUSY;
+
+	spin_lock_irqsave(&wdata->state.lock, flags);
+	wiiproto_req_accel(wdata, tmp);
+	spin_unlock_irqrestore(&wdata->state.lock, flags);
+
+	return count;
+}
+
+static DEVICE_ATTR(accelerometer, S_IRUGO | S_IWUSR, wiifs_accel_show,
+							wiifs_accel_set);
+
 static int wiimote_input_event(struct input_dev *dev, unsigned int type,
 						unsigned int code, int value)
 {
@@ -516,6 +568,9 @@  static int wiimote_hid_probe(struct hid_device *hdev,
 	ret = device_create_file(&hdev->dev, &dev_attr_rumble);
 	if (ret)
 		goto err;
+	ret = device_create_file(&hdev->dev, &dev_attr_accelerometer);
+	if (ret)
+		goto err;
 
 	ret = hid_parse(hdev);
 	if (ret) {
@@ -556,6 +611,7 @@  err:
 	device_remove_file(&hdev->dev, &dev_attr_led3);
 	device_remove_file(&hdev->dev, &dev_attr_led4);
 	device_remove_file(&hdev->dev, &dev_attr_rumble);
+	device_remove_file(&hdev->dev, &dev_attr_accelerometer);
 	wiimote_destroy(wdata);
 	return ret;
 }
@@ -571,6 +627,7 @@  static void wiimote_hid_remove(struct hid_device *hdev)
 	device_remove_file(&hdev->dev, &dev_attr_led3);
 	device_remove_file(&hdev->dev, &dev_attr_led4);
 	device_remove_file(&hdev->dev, &dev_attr_rumble);
+	device_remove_file(&hdev->dev, &dev_attr_accelerometer);
 
 	hid_hw_stop(hdev);
 	input_unregister_device(wdata->input);