From: David Herrmann <hidden> Date: 2011-07-28 16:09:03
Hi
This patch series adds support for extended wiimote features:
- force-feedback / rumble
- accelerometer
- IR cam input for up to 4 trackable items
- battery charge level
- raw eeprom access
Most of the patches just extend the current driver with trivial features. The
only new interface is the "synchronous requests" API which allows to lock the
stream for a series of requests.
For userspace tools see:
http://github.com/dvdhrm/xwiimote
tools/xwiishow.c visualizes all wiimote input and is great to test the driver.
The only missing feature is extension support which I am currently working on
and which is available in my xwiimote_kernel repository. So after this series I
will submit one last series of patches for extension support.
It would be great to get some review and feedback. The current bluez repo also
has a wiimote plugin so pairing should be as simple as with other devices now.
The next bluez release will bring it to the distros, eventually.
I've set the KernelRelease of the sysfs docs to 3.2 so there is no time pressure
for 3.1.
Regards
David
David Herrmann (16):
HID: wiimote: Support rumble device
HID: wiimote: Add sysfs rumble attribute
HID: wiimote: Add drm request
HID: wiimote: Add status and return request handlers
HID: wiimote: Reduce input syncs
HID: wiimote: Enable accelerometer on request
HID: wiimote: Parse accelerometer data
HID: wiimote: Parse IR input and report to userspace
HID: wiimote: Add missing extension DRM handlers
HID: wiimote: Add register/eeprom memory support
HID: wiimote: Helper functions for synchronous requests
HID: wiimote: Add write-register helpers
HID: wiimote: Add IR initializer
HID: wiimote: Allow userspace to control IR cam
HID: wiimote: Read wiimote battery charge level
HID: wiimote: Allow EEPROM debugfs access
Documentation/ABI/testing/sysfs-driver-hid-wiimote | 34 +
drivers/hid/hid-wiimote.c | 965 +++++++++++++++++++-
2 files changed, 993 insertions(+), 6 deletions(-)
--
1.7.6
From: David Herrmann <hidden> Date: 2011-07-28 16:09:05
This adds support for the wiimote's rumble device. Every output report can
enable and disable the rumble motor. Hence, every output report must look up our
new RUMBLE flag and make sure that it does not unintentionally toggle the
rumble motor.
Signed-off-by: David Herrmann <redacted>
---
drivers/hid/hid-wiimote.c | 44 ++++++++++++++++++++++++++++++++++++++++----
1 files changed, 40 insertions(+), 4 deletions(-)
From: David Herrmann <hidden> Date: 2011-07-28 16:09:06
Add "rumble" attribute to sysfs for every wiimote to allow userspace to control
the rumble motor of a wiimote.
Signed-off-by: David Herrmann <redacted>
---
Documentation/ABI/testing/sysfs-driver-hid-wiimote | 8 ++++
drivers/hid/hid-wiimote.c | 42 ++++++++++++++++++++
2 files changed, 50 insertions(+), 0 deletions(-)
@@ -8,3 +8,11 @@ Contact: David Herrmann <dh.herrmann@googlemail.com> Description: Make it possible to set/get current led state. Reading from it returns 0 if led is off and 1 if it is on. Writing 0 to it disables the led, writing 1 enables it.++What: /sys/bus/hid/drivers/wiimote/<dev>/rumble+Date: July 2011+KernelVersion: 3.2+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.
From: David Herrmann <hidden> Date: 2011-07-28 16:09:07
The wiimote reports data in several data reporting modes (DRM). The DRM
request makes the wiimote send data in the requested drm.
The DRM mode can be set explicitely or can be calculated by the driver. To let
the driver choose the DRM mode, pass WIIPROTO_REQ_NULL placeholder to it. This
is no valid request and is replaced with an appropriate DRM.
Currently, the driver always sets the basic DRM_K mode, but this will be
extended when further peripherals like accelerometer and IR are supported.
Signed-off-by: David Herrmann <redacted>
---
drivers/hid/hid-wiimote.c | 27 +++++++++++++++++++++++++++
1 files changed, 27 insertions(+), 0 deletions(-)
From: David Herrmann <hidden> Date: 2011-07-28 16:09:08
The wiimote resets the current drm when an extension is plugged in.
Fortunately, it also sends a status report in this situation so we just
reset the drm on every status report to keep the drm consistent.
Also handle return reports from the wiimote which indicate success and
failure of requests that we've sent.
Signed-off-by: David Herrmann <redacted>
---
drivers/hid/hid-wiimote.c | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)
@@ -381,6 +383,26 @@ static void handler_keys(struct wiimote_data *wdata, const __u8 *payload)input_sync(wdata->input);}+staticvoidhandler_status(structwiimote_data*wdata,const__u8*payload)+{+handler_keys(wdata,payload);++/* on status reports the drm is reset so we need to resend the drm */+wiiproto_req_drm(wdata,0);+}++staticvoidhandler_return(structwiimote_data*wdata,const__u8*payload)+{+__u8err=payload[3];+__u8cmd=payload[2];++handler_keys(wdata,payload);++if(err)+hid_warn(wdata->hdev,"Remote error %hhu on req %hhu\n",err,+cmd);+}+structwiiproto_handler{__u8id;size_tsize;
From: David Herrmann <hidden> Date: 2011-07-28 16:09:10
To avoid multiple input syncs to be sent when parsing multiple
peripheral inputs from the wiimote in future, we send the input-sync
after handling all input events and not after parsing each value. This avoids
waking up user-space handlers multiple times on a single incoming package.
Signed-off-by: David Herrmann <redacted>
---
drivers/hid/hid-wiimote.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
From: David Herrmann <hidden> Date: 2011-07-28 16:09:11
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 <redacted>
---
Documentation/ABI/testing/sysfs-driver-hid-wiimote | 8 +++
drivers/hid/hid-wiimote.c | 57 ++++++++++++++++++++
2 files changed, 65 insertions(+), 0 deletions(-)
@@ -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.
From: David Herrmann <hidden> Date: 2011-07-28 16:09:13
Add parser functions for accelerometer data reported by the wiimote. The data is
almost always reported in the same format, so we can use a single handler.
However, an own handler function is created for each DRM-mode because when IR
and extension support is added, each of them is parsed differently.
Also set the appropriate DRM including accelerometer data on DRM requests to
actually retrieve the accelerometer data.
Data is reported to userspace as ABS_X/Y/Z values. The values are between -500
and 500 and 0 means no acceleration. See also userspace xwiimote library for
data parsing.
Signed-off-by: David Herrmann <redacted>
---
drivers/hid/hid-wiimote.c | 102 ++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 101 insertions(+), 1 deletions(-)
From: David Herrmann <hidden> Date: 2011-07-28 16:09:14
A wiimote sends IR pointing information for up to 4 trackable IR lights. If less
lights are visible, the missing lights report max value. This patch adds parser
functions for IR input and reports this via ABS_HAT*XY values to the input
subsystem.
The IR cam can be in four states: off, basic, extended, full
The DRM chooser automatically chosses an DRM that matches the current IR cam
state so no information is lost.
Signed-off-by: David Herrmann <redacted>
---
drivers/hid/hid-wiimote.c | 101 +++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 97 insertions(+), 4 deletions(-)
From: David Herrmann <hidden> Date: 2011-07-28 16:09:15
If an extension is connected the wiimote may report data though DRMs that
contain extension data. This adds handlers for these DRMs but discards extension
data since we do not support it, yet.
It prints a warning to kernel log if an unhandled report is catched. Since we
handle all requests now, this should never happen, though.
Signed-off-by: David Herrmann <redacted>
---
drivers/hid/hid-wiimote.c | 29 ++++++++++++++++++++++++++++-
1 files changed, 28 insertions(+), 1 deletions(-)
From: David Herrmann <hidden> Date: 2011-07-28 16:09:18
The wiimote allows direct access to its memory mapped registers and
internal eeprom. This adds support to access this memory and handle
memory events.
There are two macros which wrap up the memory access functions to avoid
accidentally overwriting sensitive eeprom data because a boolean value
was wrongly set.
Signed-off-by: David Herrmann <redacted>
---
drivers/hid/hid-wiimote.c | 71 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 71 insertions(+), 0 deletions(-)
From: David Herrmann <hidden> Date: 2011-07-28 16:09:18
To initialize wiimote peripherals, the stream to the wiimote must be held
exclusively by the initializer, otherwise the initialization will fail. Many
initializations require multiple memory requests to be sent synchronously so we
need a way to lock the stream and release it when we are done.
This adds several helper functions which allow to lock the stream, then send
requests, wait for the answers and release the stream again.
When holding the lock, the function may sleep and interrupted by signals.
Also it returns after a short timeout so userspace shouldn't notice long
delays.
Signed-off-by: David Herrmann <redacted>
---
drivers/hid/hid-wiimote.c | 56 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 56 insertions(+), 0 deletions(-)
@@ -116,6 +124,52 @@ static __u16 wiiproto_keymap[] = {#define dev_to_wii(pdev) hid_get_drvdata(container_of(pdev, struct hid_device, \dev))+/* requires the state.lock spinlock to be held */+staticinlineboolwiimote_cmd_pending(structwiimote_data*wdata,intcmd,+__u32opt)+{+returnwdata->state.cmd==cmd&&wdata->state.opt==opt;+}++/* requires the state.lock spinlock to be held */+staticinlinevoidwiimote_cmd_complete(structwiimote_data*wdata)+{+wdata->state.cmd=WIIPROTO_REQ_NULL;+complete(&wdata->state.ready);+}++staticinlineintwiimote_cmd_acquire(structwiimote_data*wdata)+{+returnmutex_lock_interruptible(&wdata->state.sync)?-ERESTARTSYS:0;+}++/* requires the state.lock spinlock to be held */+staticinlinevoidwiimote_cmd_set(structwiimote_data*wdata,intcmd,+__u32opt)+{+INIT_COMPLETION(wdata->state.ready);+wdata->state.cmd=cmd;+wdata->state.opt=opt;+}++staticinlinevoidwiimote_cmd_release(structwiimote_data*wdata)+{+mutex_unlock(&wdata->state.sync);+}++staticinlineintwiimote_cmd_wait(structwiimote_data*wdata)+{+intret;++ret=wait_for_completion_interruptible_timeout(&wdata->state.ready,HZ);+if(ret<0)+return-ERESTARTSYS;+elseif(ret==0)+return-EIO;+else+return0;+}+staticssize_twiimote_hid_send(structhid_device*hdev,__u8*buffer,size_tcount){
From: David Herrmann <hidden> Date: 2011-07-28 16:09:20
Add helpers to synchronously write registers of the wiimote. This is heavily
used by initialization functions for wiimote peripherals.
Signed-off-by: David Herrmann <redacted>
---
drivers/hid/hid-wiimote.c | 27 ++++++++++++++++++++++++++-
1 files changed, 26 insertions(+), 1 deletions(-)
From: David Herrmann <hidden> Date: 2011-07-28 16:09:22
The wiimote IR cam needs a fairly complex initialization sequence. This adds a
helper function that performs IR initialization synchronously.
Signed-off-by: David Herrmann <redacted>
---
drivers/hid/hid-wiimote.c | 134 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 134 insertions(+), 0 deletions(-)
@@ -443,6 +467,116 @@ static int wiimote_cmd_write(struct wiimote_data *wdata, __u32 offset,returnret;}+staticintwiimote_init_ir(structwiimote_data*wdata,__u16mode)+{+intret;+unsignedlongflags;+__u8format=0;+staticconst__u8data_enable[]={0x01};+staticconst__u8data_sens1[]={0x02,0x00,0x00,0x71,0x01,+0x00,0xaa,0x00,0x64};+staticconst__u8data_sens2[]={0x63,0x03};+staticconst__u8data_fin[]={0x08};++ret=wiimote_cmd_acquire(wdata);+if(ret)+returnret;++spin_lock_irqsave(&wdata->state.lock,flags);++if(mode==(wdata->state.flags&WIIPROTO_FLAGS_IR)){+spin_unlock_irqrestore(&wdata->state.lock,flags);+gotounlock;+}++if(mode==0){+wdata->state.flags&=~WIIPROTO_FLAGS_IR;+wiiproto_req_ir1(wdata,0);+wiiproto_req_ir2(wdata,0);+wiiproto_req_drm(wdata,WIIPROTO_REQ_NULL);+spin_unlock_irqrestore(&wdata->state.lock,flags);+gotounlock;+}++/* send PIXEL CLOCK ENABLE cmd first */+wiimote_cmd_set(wdata,WIIPROTO_REQ_IR1,0);+wiiproto_req_ir1(wdata,0x06);++spin_unlock_irqrestore(&wdata->state.lock,flags);++ret=wiimote_cmd_wait(wdata);+if(ret)+gotounlock;+if(wdata->state.cmd_err){+ret=-EIO;+gotounlock;+}++/* enable IR LOGIC then */+spin_lock_irqsave(&wdata->state.lock,flags);+wiimote_cmd_set(wdata,WIIPROTO_REQ_IR2,0);+wiiproto_req_ir2(wdata,0x06);+spin_unlock_irqrestore(&wdata->state.lock,flags);++ret=wiimote_cmd_wait(wdata);+if(ret)+gotounlock;+if(wdata->state.cmd_err){+ret=-EIO;+gotounlock;+}++/* enable IR cam but do not make it send data, yet */+ret=wiimote_cmd_write(wdata,0xb00030,data_enable,+sizeof(data_enable));+if(ret)+gotounlock;++/* write first sensitivity block */+ret=wiimote_cmd_write(wdata,0xb00000,data_sens1,+sizeof(data_sens1));+if(ret)+gotounlock;++/* write second sensitivity block */+ret=wiimote_cmd_write(wdata,0xb0001a,data_sens2,+sizeof(data_sens2));+if(ret)+gotounlock;++/* put IR cam into desired state */+switch(mode){+caseWIIPROTO_FLAG_IR_FULL:+format=5;+break;+caseWIIPROTO_FLAG_IR_EXT:+format=3;+break;+caseWIIPROTO_FLAG_IR_BASIC:+format=1;+break;+}+ret=wiimote_cmd_write(wdata,0xb00033,&format,sizeof(format));+if(ret)+gotounlock;++/* make IR cam send data */+ret=wiimote_cmd_write(wdata,0xb00030,data_fin,sizeof(data_fin));+if(ret)+gotounlock;++/* request new DRM mode compatible to IR mode */+spin_lock_irqsave(&wdata->state.lock,flags);+wdata->state.flags&=~WIIPROTO_FLAGS_IR;+wdata->state.flags|=mode;+wiiproto_req_drm(wdata,0);+spin_unlock_irqrestore(&wdata->state.lock,flags);++unlock:+wiimote_cmd_release(wdata);+returnret;+}+#define wiifs_led_show_set(num) \staticssize_twiifs_led_show_##num(structdevice*dev,\structdevice_attribute*attr,char*buf)\
From: David Herrmann <hidden> Date: 2011-07-28 16:09:23
This adds a new sysfs attribute to wiimotes which allows userspace to enable or
disable the IR cam and set it into the desired mode.
Disabling IR saves lots of energy on the wiimote, so it is not enabled by
default.
Signed-off-by: David Herrmann <redacted>
---
Documentation/ABI/testing/sysfs-driver-hid-wiimote | 10 +++
drivers/hid/hid-wiimote.c | 74 ++++++++++++++++++++
2 files changed, 84 insertions(+), 0 deletions(-)
@@ -24,3 +24,13 @@ 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.++What: /sys/bus/hid/drivers/wiimote/<dev>/ir+Date: July 2011+KernelVersion: 3.2+Contact: David Herrmann <dh.herrmann@googlemail.com>+Description: Reading this attribute returns the current status of the IR+ cam of the wiimote. It can be one of: off, basic, extended or+ full.+ Writing one of these strings to the file tries to put the IR+ cam into the desired state.
@@ -700,6 +700,75 @@ static ssize_t wiifs_accel_set(struct device *dev,staticDEVICE_ATTR(accelerometer,S_IRUGO|S_IWUSR,wiifs_accel_show,wiifs_accel_set);+staticssize_twiifs_ir_show(structdevice*dev,structdevice_attribute*attr,+char*buf)+{+structwiimote_data*wdata=dev_to_wii(dev);+unsignedlongflags;+constchar*mode;+__u8flag;++if(!atomic_read(&wdata->ready))+return-EBUSY;++spin_lock_irqsave(&wdata->state.lock,flags);+flag=wdata->state.flags&WIIPROTO_FLAGS_IR;+spin_unlock_irqrestore(&wdata->state.lock,flags);++switch(flag){+caseWIIPROTO_FLAG_IR_FULL:+mode="full";+break;+caseWIIPROTO_FLAG_IR_EXT:+mode="extended";+break;+caseWIIPROTO_FLAG_IR_BASIC:+mode="basic";+break;+default:+mode="off";+break;+}+++returnsprintf(buf,"%s\n",mode);+}++staticssize_twiifs_ir_set(structdevice*dev,structdevice_attribute*attr,+constchar*buf,size_tcount)+{+structwiimote_data*wdata=dev_to_wii(dev);+intret;+__u8mode;++if(count==0)+return-EINVAL;++if(!strncasecmp("basic",buf,5))+mode=WIIPROTO_FLAG_IR_BASIC;+elseif(!strncasecmp("extended",buf,8))+mode=WIIPROTO_FLAG_IR_EXT;+elseif(!strncasecmp("full",buf,4))+mode=WIIPROTO_FLAG_IR_FULL;+elseif(!strncasecmp("off",buf,3))+mode=0;+else+return-EINVAL;++if(!atomic_read(&wdata->ready))+return-EBUSY;+/* smp_rmb: Make sure wdata->xy is available when wdata->ready is 1 */+smp_rmb();++ret=wiimote_init_ir(wdata,mode);+if(ret<0)+returnret;++returncount;+}++staticDEVICE_ATTR(ir,S_IRUGO|S_IWUSR,wiifs_ir_show,wiifs_ir_set);+staticintwiimote_input_event(structinput_dev*dev,unsignedinttype,unsignedintcode,intvalue){
@@ -1077,6 +1146,9 @@ static int wiimote_hid_probe(struct hid_device *hdev,ret=device_create_file(&hdev->dev,&dev_attr_accelerometer);if(ret)gotoerr;+ret=device_create_file(&hdev->dev,&dev_attr_ir);+if(ret)+gotoerr;ret=hid_parse(hdev);if(ret){
From: David Herrmann <hidden> Date: 2011-07-28 16:09:24
This adds a new sysfs file for wiimotes which returns the current battery charge
level of the device. Since this information is not sent by the wiimote
continously, we need to explicitely request it.
Signed-off-by: David Herrmann <redacted>
---
Documentation/ABI/testing/sysfs-driver-hid-wiimote | 8 +++
drivers/hid/hid-wiimote.c | 56 ++++++++++++++++++++
2 files changed, 64 insertions(+), 0 deletions(-)
@@ -34,3 +34,11 @@ Description: Reading this attribute returns the current status of the IR full. Writing one of these strings to the file tries to put the IR cam into the desired state.++What: /sys/bus/hid/drivers/wiimote/<dev>/battery+Date: July 2011+KernelVersion: 3.2+Contact: David Herrmann <dh.herrmann@googlemail.com>+Description: Readonly attribute which returns the current battery charge+ level. An integer between 0 and 255 is returned, 0 means empty+ and 255 full.
@@ -700,6 +713,39 @@ static ssize_t wiifs_accel_set(struct device *dev,staticDEVICE_ATTR(accelerometer,S_IRUGO|S_IWUSR,wiifs_accel_show,wiifs_accel_set);+staticssize_twiifs_battery_show(structdevice*dev,+structdevice_attribute*attr,char*buf)+{+structwiimote_data*wdata=dev_to_wii(dev);+unsignedlongflags;+intstate,ret;++if(!atomic_read(&wdata->ready))+return-EBUSY;+/* smp_rmb: Make sure wdata->xy is available when wdata->ready is 1 */+smp_rmb();++ret=wiimote_cmd_acquire(wdata);+if(ret)+returnret;++spin_lock_irqsave(&wdata->state.lock,flags);+wiimote_cmd_set(wdata,WIIPROTO_REQ_SREQ,0);+wiiproto_req_status(wdata);+spin_unlock_irqrestore(&wdata->state.lock,flags);++ret=wiimote_cmd_wait(wdata);+state=wdata->state.cmd_battery;+wiimote_cmd_release(wdata);++if(ret)+returnret;++returnsprintf(buf,"%d\n",state);+}++staticDEVICE_ATTR(battery,S_IRUGO,wiifs_battery_show,NULL);+staticssize_twiifs_ir_show(structdevice*dev,structdevice_attribute*attr,char*buf){
@@ -874,6 +920,11 @@ static void handler_status(struct wiimote_data *wdata, const __u8 *payload)/* on status reports the drm is reset so we need to resend the drm */wiiproto_req_drm(wdata,0);++if(wiimote_cmd_pending(wdata,WIIPROTO_REQ_SREQ,0)){+wdata->state.cmd_battery=payload[5];+wiimote_cmd_complete(wdata);+}}staticvoidhandler_data(structwiimote_data*wdata,const__u8*payload)
@@ -1149,6 +1200,9 @@ static int wiimote_hid_probe(struct hid_device *hdev,ret=device_create_file(&hdev->dev,&dev_attr_ir);if(ret)gotoerr;+ret=device_create_file(&hdev->dev,&dev_attr_battery);+if(ret)+gotoerr;ret=hid_parse(hdev);if(ret){
From: David Herrmann <hidden> Date: 2011-07-28 16:09:26
Add "eeprom" file to debugfs for every wiimote. Reading from this file allows to
read the internal EEPROM of a wiimote. Writing to eeprom is currently not
allowed and eeprom memory hasn't been reverse-engineered, yet.
This file shall only be used for debugging. Reading large blocks from this file
may block other access to the wiimote, hence, each block has been limited to 16
bytes to avoid large memory transfers while holding the wiimote lock.
Signed-off-by: David Herrmann <redacted>
---
drivers/hid/hid-wiimote.c | 131 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 131 insertions(+), 0 deletions(-)
@@ -1222,6 +1350,8 @@ static int wiimote_hid_probe(struct hid_device *hdev,gotoerr_stop;}+wiimote_debugfs_init(wdata);+/* smp_wmb: Write wdata->xy first before wdata->ready is set to 1 */smp_wmb();atomic_set(&wdata->ready,1);
From: Oliver Neukum <hidden> Date: 2011-07-28 18:14:11
Am Donnerstag, 28. Juli 2011, 18:08:27 schrieb David Herrmann:
+static void handler_accel(struct wiimote_data *wdata, const __u8 *payload)
+{
+ __u16 x, y, z;
+
+ if (!(wdata->state.flags & WIIPROTO_FLAG_ACCEL))
+ return;
+
+ /*
+ * payload is: BB BB XX YY ZZ
+ * Buttons data contains LSBs
+ */
+
+ x = payload[2] << 2;
+ y = payload[3] << 2;
+ z = payload[4] << 2;
+
+ x |= (payload[0] >> 5) & 0x3;
+ y |= (payload[1] >> 4) & 0x2;
+ z |= (payload[1] >> 5) & 0x2;
Could you make the comments a bit clearer. Those last lines are impossible
to understand.
Regards
Oliver
--
- - -
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstraße 5
90409 Nürnberg
Germany
- - -
--
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
From: Oliver Neukum <hidden> Date: 2011-07-28 18:14:11
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
--
- - -
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstraße 5
90409 Nürnberg
Germany
- - -
--
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
Are you sure these buffers nned not be safe for DMA?
Regards
Oliver
--
- - -
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstraße 5
90409 Nürnberg
Germany
- - -
--
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
Are you sure these buffers nned not be safe for DMA?
wiimote_queue() allocates a buffer for every request. This should be
fine for DMA then. All wiimote_req_*() functions work the same way.
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
From: David Herrmann <hidden> Date: 2011-07-28 18:51:38
On Thu, Jul 28, 2011 at 8:12 PM, Oliver Neukum [off-list ref] wrote:
Am Donnerstag, 28. Juli 2011, 18:08:27 schrieb David Herrmann:
quoted
+static void handler_accel(struct wiimote_data *wdata, const __u8 *payload)
+{
+ __u16 x, y, z;
+
+ if (!(wdata->state.flags & WIIPROTO_FLAG_ACCEL))
+ return;
+
+ /*
+ * payload is: BB BB XX YY ZZ
+ * Buttons data contains LSBs
+ */
+
+ x = payload[2] << 2;
+ y = payload[3] << 2;
+ z = payload[4] << 2;
+
+ x |= (payload[0] >> 5) & 0x3;
+ y |= (payload[1] >> 4) & 0x2;
+ z |= (payload[1] >> 5) & 0x2;
Could you make the comments a bit clearer. Those last lines are impossible
to understand.
The LSBs are encoded in the "BB BB" data and I am extracting them. I
have documented the whole protocol in a separated document but if it
is common practice to add those comments to the code, I will add it in
the next version.
X, Y and Z data of accelerometer have 10 bits of precision. The data
is reported as 5 bytes:
BB BB XX YY ZZ
The first two bytes also contain the button data but there are 5
additional bits in the BB BB data that contain the LSBs of the
accelerometer.
XX YY ZZ are the upper 8 bits of the three accelerometer values.
BB BB have 8 bits. Bit 6 and 7 of first byte are LSBs of X value.
Bit 6 of second byte is LSB of Y value. Bit 7 of second byte is LSB of
Z value. The 10th bit of Y and Z are no available and set to 0.
As a table:
1st byte BB
Bit: 1 2 3 4 5 6 7 8
---------------
B B B B B X X B
2nd byte BB
Bit: 1 2 3 4 5 6 7 8
---------------
B B B B B Y Z B
B is button data and ignored here. handler_keys() takes care of them.
Regards
Oliver
Thanks for your review.
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
From: David Herrmann <hidden> Date: 2011-07-28 19:01:35
On Thu, Jul 28, 2011 at 8:08 PM, Oliver Neukum [off-list ref] wrote:
Am Donnerstag, 28. Juli 2011, 18:08:26 schrieb David Herrmann:
quoted
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
From: Oliver Neukum <hidden> Date: 2011-07-28 19:29:59
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
--
- - -
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstraße 5
90409 Nürnberg
Germany
- - -
--
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
+static void handler_accel(struct wiimote_data *wdata, const __u8 *payload)
+{
+ __u16 x, y, z;
+
+ if (!(wdata->state.flags & WIIPROTO_FLAG_ACCEL))
+ return;
+
+ /*
+ * payload is: BB BB XX YY ZZ
+ * Buttons data contains LSBs
+ */
+
+ x = payload[2] << 2;
+ y = payload[3] << 2;
+ z = payload[4] << 2;
+
+ x |= (payload[0] >> 5) & 0x3;
+ y |= (payload[1] >> 4) & 0x2;
+ z |= (payload[1] >> 5) & 0x2;
Could you make the comments a bit clearer. Those last lines are impossible
to understand.
The LSBs are encoded in the "BB BB" data and I am extracting them. I
have documented the whole protocol in a separated document but if it
is common practice to add those comments to the code, I will add it in
the next version.
In some cases, the usual way is to put a brief description of the
protocol/data structures mandated by hardware in the comments at the very
beginning of the driver file.
The description below seems like a perfect fit for such purpose. If you
have much more elaborate description, it should probably go into
Documentation/.
Thanks,
--
Jiri Kosina
SUSE Labs
--
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