2007-05-29 21:11:42

by Jan Kratochvil

[permalink] [raw]
Subject: [PATCH] Support for controlling leds on xbox 360 pad.

Hello,
this patch is against current input tree.

Xbox360 pad has four leds, which forms a circle. Unfortunately the leds itself
are not independent, and we can't control them directle, but rather through
sending commands which have predefined meaning (like turn first on, others off)
This patch allows us to send these commands via leds subsystem. Commands itself
are described here: http://www.free60.org/wiki/Gamepad.

Led subsystem allows us to set brightness, but there is nothing like brightness
on this device. So brightness is actually interpreted as the command (only
values between 0 and 14 are accepted).

So this patch uses led subystem in such way that it makes led in
/sys/class/leds/xpad:[0-9][0-9]/ for each attached xbox 360 pad.

Signed-off-by: Jan Kratochvil <[email protected]>

---
drivers/input/joystick/xpad.c | 34 ++++-------
drivers/input/joystick/xpad.h | 35 ++++++++++++
drivers/leds/Kconfig | 7 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-xpad.c | 123 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 179 insertions(+), 21 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 664c765..4322ec9 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -79,6 +79,8 @@
#include <linux/smp_lock.h>
#include <linux/usb/input.h>

+#include "xpad.h"
+
#define DRIVER_VERSION "v0.0.6"
#define DRIVER_AUTHOR "Marko Friedemann <[email protected]>"
#define DRIVER_DESC "X-Box pad driver"
@@ -184,26 +186,6 @@ static struct usb_device_id xpad_table [

MODULE_DEVICE_TABLE (usb, xpad_table);

-struct usb_xpad {
- struct input_dev *dev; /* input device interface */
- struct usb_device *udev; /* usb device */
-
- struct urb *irq_in; /* urb for interrupt in report */
- unsigned char *idata; /* input data */
- dma_addr_t idata_dma;
-
-#ifdef CONFIG_JOYSTICK_XPAD_FF
- struct urb *irq_out; /* urb for interrupt out report */
- unsigned char *odata; /* output data */
- dma_addr_t odata_dma;
-#endif
-
- char phys[65]; /* physical device path */
-
- int dpad_mapping; /* map d-pad to buttons or to axes */
- int xtype; /* type of xbox device */
-};
-
/*
* xpad_process_packet
*
@@ -384,6 +366,7 @@ int xpad_play_effect(struct input_dev *d
if (effect->type == FF_RUMBLE) {
__u16 strong = effect->u.rumble.strong_magnitude;
__u16 weak = effect->u.rumble.weak_magnitude;
+ mutex_lock(&xpad->odata_mutex);
xpad->odata[0] = 0x00;
xpad->odata[1] = 0x08;
xpad->odata[2] = 0x00;
@@ -393,6 +376,7 @@ int xpad_play_effect(struct input_dev *d
xpad->odata[6] = 0x00;
xpad->odata[7] = 0x00;
usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+ mutex_unlock(&xpad->odata_mutex);
}

return 0;
@@ -408,8 +392,10 @@ static int xpad_init_ff(struct usb_inter

xpad->odata = usb_buffer_alloc(xpad->udev, XPAD_PKT_LEN,
GFP_ATOMIC, &xpad->odata_dma );
- if (!xpad->idata)
+ if (!xpad->odata)
goto fail1;
+
+ mutex_init(&xpad->odata_mutex);

xpad->irq_out = usb_alloc_urb(0, GFP_KERNEL);
if (!xpad->irq_out)
@@ -568,6 +554,9 @@ static int xpad_probe(struct usb_interfa
if (error)
goto fail2;

+ if (xpad->xtype == XTYPE_XBOX360)
+ xpad_led_probe(xpad);
+
ep_irq_in = &intf->cur_altsetting->endpoint[0].desc;
usb_fill_int_urb(xpad->irq_in, udev,
usb_rcvintpipe(udev, ep_irq_in->bEndpointAddress),
@@ -597,6 +586,9 @@ static void xpad_disconnect(struct usb_i

usb_set_intfdata(intf, NULL);
if (xpad) {
+ if (xpad->xtype == XTYPE_XBOX360)
+ xpad_led_disconnect(xpad);
+
input_unregister_device(xpad->dev);
xpad_deinit_ff(xpad);
usb_free_urb(xpad->irq_in);
diff --git a/drivers/input/joystick/xpad.h b/drivers/input/joystick/xpad.h
new file mode 100644
index 0000000..487a129
--- /dev/null
+++ b/drivers/input/joystick/xpad.h
@@ -0,0 +1,35 @@
+#ifndef __INPUT_XPAD_H_INCLUDED
+#define __INPUT_XPAD_H_INCLUDED
+
+#include <linux/usb/input.h>
+
+struct usb_xpad {
+ struct input_dev *dev; /* input device interface */
+ struct usb_device *udev; /* usb device */
+
+ struct urb *irq_in; /* urb for interrupt in report */
+ unsigned char *idata; /* input data */
+ dma_addr_t idata_dma;
+
+#ifdef CONFIG_JOYSTICK_XPAD_FF
+ struct urb *irq_out; /* urb for interrupt out report */
+ unsigned char *odata; /* output data */
+ dma_addr_t odata_dma;
+ struct mutex odata_mutex;
+#endif
+
+ char phys[65]; /* physical device path */
+
+ int dpad_mapping; /* map d-pad to buttons or to axes */
+ int xtype; /* type of xbox device */
+};
+
+#if defined(CONFIG_LEDS_XBOX360) || defined(CONFIG_LEDS_XBOX360_MODULE)
+int xpad_led_probe(struct usb_xpad *xpad);
+int xpad_led_disconnect(struct usb_xpad *xpad);
+#else
+inline int xpad_led_probe(struct usb_xpad *) { return 0; }
+inline int xpad_led_disconnect(struct usb_xpad *) { return 0; }
+#endif
+
+#endif /* __INPUT_XPAD_H_INCLUDED */
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 80acd08..c82b233 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -94,6 +94,13 @@ config LEDS_COBALT
help
This option enables support for the front LED on Cobalt Server

+config LEDS_XBOX360
+ tristate "LED Support for Xbox360 controller 'BigX' LED"
+ depends on LEDS_CLASS && JOYSTICK_XPAD_FF
+ help
+ This option enables support for the LED which surrounds the Big X on
+ XBox 360 controller.
+
comment "LED Triggers"

config LEDS_TRIGGERS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index aa2c18e..1219df0 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_LEDS_NET48XX) += leds-net4
obj-$(CONFIG_LEDS_WRAP) += leds-wrap.o
obj-$(CONFIG_LEDS_H1940) += leds-h1940.o
obj-$(CONFIG_LEDS_COBALT) += leds-cobalt.o
+obj-$(CONFIG_LEDS_XBOX360) += leds-xpad.o

# LED Triggers
obj-$(CONFIG_LEDS_TRIGGER_TIMER) += ledtrig-timer.o
diff --git a/drivers/leds/leds-xpad.c b/drivers/leds/leds-xpad.c
new file mode 100644
index 0000000..8dcda4a
--- /dev/null
+++ b/drivers/leds/leds-xpad.c
@@ -0,0 +1,123 @@
+/*
+ * LEDs driver for Xbox360 controller
+ *
+ * Copyright (C) 2007 Jan Kratochvil <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include "../input/joystick/xpad.h"
+
+struct xpad_led {
+ struct led_classdev cdev;
+
+ unsigned int id;
+ struct usb_xpad * xpad;
+ struct list_head node;
+};
+
+static LIST_HEAD(xpad_led_list);
+
+static void xpad_send_command(int command, struct usb_xpad *xpad)
+{
+ if (command >= 0 && command < 14) {
+ mutex_lock(&xpad->odata_mutex);
+ xpad->odata[0] = 0x01;
+ xpad->odata[1] = 0x03;
+ xpad->odata[2] = command;
+ usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+ mutex_unlock(&xpad->odata_mutex);
+ }
+}
+
+static void xpad_led_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ struct xpad_led *led_dev =
+ container_of(led_cdev, struct xpad_led, cdev);
+
+ xpad_send_command(value, led_dev->xpad);
+}
+
+int xpad_led_probe(struct usb_xpad *xpad)
+{
+ int i = 0;
+ struct xpad_led *pos_led;
+ struct xpad_led *new_led;
+ int error = -ENOMEM;
+ char *name;
+
+ list_for_each_entry(pos_led, &xpad_led_list, node) {
+ if (pos_led->id == i)
+ i++;
+ else
+ break;
+ }
+
+ if (i > 99)
+ goto fail1;
+
+ new_led = kzalloc(sizeof(struct xpad_led), GFP_KERNEL);
+ if (!new_led)
+ goto fail1;
+
+ new_led->id = i;
+
+ name = kzalloc(sizeof(char)*8, GFP_KERNEL);
+ if (!name)
+ goto fail2;
+
+ strcpy(name, "xpad:");
+ name[5] = '0' + i / 10;
+ name[6] = '0' + i % 10;
+ name[7] = 0;
+
+ new_led->cdev.name = name;
+ new_led->cdev.brightness_set = xpad_led_set;
+ new_led->xpad = xpad;
+
+ list_add_tail(&new_led->node, &pos_led->node);
+
+ if (led_classdev_register(&xpad->udev->dev, &new_led->cdev))
+ goto fail3;
+
+
+ xpad_send_command( (i % 4) + 2, xpad);
+
+ return 0;
+
+fail3:
+ kfree(new_led->cdev.name);
+fail2:
+ kfree(new_led);
+fail1:
+ return error;
+}
+EXPORT_SYMBOL_GPL(xpad_led_probe);
+
+int xpad_led_disconnect(struct usb_xpad *xpad)
+{
+ struct xpad_led *led;
+ struct list_head *pos, *tmp;
+ list_for_each_safe(pos, tmp, &xpad_led_list) {
+ led = list_entry(pos, struct xpad_led, node);
+ if (led->xpad == xpad) {
+ led_classdev_unregister(&led->cdev);
+ list_del(pos);
+ kfree(led->cdev.name);
+ kfree(led);
+ break;
+ }
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(xpad_led_disconnect);
+
+MODULE_AUTHOR("Jan Kratochvil <[email protected]>");
+MODULE_DESCRIPTION("Xbox 360 LED driver");
+MODULE_LICENSE("GPL");
--
1.4.3.4



2007-05-29 21:29:58

by Jan Kratochvil

[permalink] [raw]
Subject: Re: [PATCH] Support for controlling leds on xbox 360 pad.

Hello,
I have question, probably for Richard. Why is
/sys/class/leds/whatsoever/brightness mode set to 0644? Is it really necessary?
I feel like I'll be happy to allow anybody to change the state of this led. (Ok
this maybe doesn't apply to other leds)

> So this patch uses led subystem in such way that it makes led in
> /sys/class/leds/xpad:[0-9][0-9]/ for each attached xbox 360 pad.
>

I have the impression that the led subsystem doesn't actually count with
possibility to have more then one device of one type, right? In my case there
can by lots of pads, so I made it on my own, using counter. Is it ok?

Thanks
Jan Kratochvil

2007-05-29 21:41:41

by Richard Purdie

[permalink] [raw]
Subject: Re: [PATCH] Support for controlling leds on xbox 360 pad.

On Tue, 2007-05-29 at 23:29 +0200, Jan Kratochvil wrote:
> I have question, probably for Richard. Why is
> /sys/class/leds/whatsoever/brightness mode set to 0644? Is it really necessary?
> I feel like I'll be happy to allow anybody to change the state of this led. (Ok
> this maybe doesn't apply to other leds)

Permissions management of the LEDs is outside the scope of kernel. If
you need users to have access to them, change the permissions in
userspace to grant access.

> > So this patch uses led subystem in such way that it makes led in
> > /sys/class/leds/xpad:[0-9][0-9]/ for each attached xbox 360 pad.
>
> I have the impression that the led subsystem doesn't actually count with
> possibility to have more then one device of one type, right? In my case there
> can by lots of pads, so I made it on my own, using counter. Is it ok?

It leaves it up to the driver concerned. In your case it makes sense so
counting is fine. Don't put the colon in though, just have it as xpad0
etc. as the name. Anything after the first colon is defined as an
optional colour (see Documentation/leds-class.txt).

Richard

2007-05-30 03:50:38

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Support for controlling leds on xbox 360 pad.

On Tuesday 29 May 2007 17:41, Richard Purdie wrote:
> On Tue, 2007-05-29 at 23:29 +0200, Jan Kratochvil wrote:
> >    I have question, probably for Richard. Why is
> > /sys/class/leds/whatsoever/brightness mode set to 0644? Is it really necessary?
> > I feel like I'll be happy to allow anybody to change the state of this led. (Ok
> > this maybe doesn't apply to other leds)
>
> Permissions management of the LEDs is outside the scope of kernel. If
> you need users to have access to them, change the permissions in
> userspace to grant access.
>

I will also venture to say that you only want to grant access to
the user currently logged on console, not any random user logged
in over the network so you really want to dynamically manage
premissions. The kernel just provides sensible defaults.

--
Dmitry

2007-05-30 14:56:40

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Support for controlling leds on xbox 360 pad.

On 5/29/07, Richard Purdie <[email protected]> wrote:
> On Tue, 2007-05-29 at 23:29 +0200, Jan Kratochvil wrote:
> >
> > I have the impression that the led subsystem doesn't actually count with
> > possibility to have more then one device of one type, right? In my case there
> > can by lots of pads, so I made it on my own, using counter. Is it ok?
>
> It leaves it up to the driver concerned. In your case it makes sense so
> counting is fine. Don't put the colon in though, just have it as xpad0
> etc. as the name. Anything after the first colon is defined as an
> optional colour (see Documentation/leds-class.txt).
>

Richard,

Do you think it makes sense to split the driver (and Kconfig options)
between input and leds directories as Jan had done? I know that I
prefer to keep anything input related in input directory because it
makes my life as maintainer easier and I bet you prefer the same but
it does not always work well for multifunction devices or devices
using several subsystems...
--
Dmitry

2007-05-30 16:34:09

by Richard Purdie

[permalink] [raw]
Subject: Re: [PATCH] Support for controlling leds on xbox 360 pad.

Hi Dmitry,

On Wed, 2007-05-30 at 10:56 -0400, Dmitry Torokhov wrote:
> Do you think it makes sense to split the driver (and Kconfig options)
> between input and leds directories as Jan had done? I know that I
> prefer to keep anything input related in input directory because it
> makes my life as maintainer easier and I bet you prefer the same but
> it does not always work well for multifunction devices or devices
> using several subsystems...

In ideal world having the code in drivers/leds would be preferred but if
you want to add the LED code directly to the input driver in this case I
have no objection and it probably makes sense. The LED code is already
spread about a bit anyway...

Cheers,

Richard

2007-05-30 21:38:24

by Jan Kratochvil

[permalink] [raw]
Subject: Re: [PATCH] Support for controlling leds on xbox 360 pad.

Hi,

On Wed, 30 May 2007, Richard Purdie wrote:
> On Wed, 2007-05-30 at 10:56 -0400, Dmitry Torokhov wrote:
>> Do you think it makes sense to split the driver (and Kconfig options)
>> between input and leds directories as Jan had done? I know that I
>> prefer to keep anything input related in input directory because it
>> makes my life as maintainer easier and I bet you prefer the same but
>> it does not always work well for multifunction devices or devices
>> using several subsystems...
>
> In ideal world having the code in drivers/leds would be preferred but if
> you want to add the LED code directly to the input driver in this case I
> have no objection and it probably makes sense. The LED code is already
> spread about a bit anyway...

I wasn't happy about having this splited in two subsystems either. So I'll
rewrite it to use led functionality directly from drivers/input/joystick/xpad.c.

Thanks for comments
Jan

2007-05-31 15:33:45

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] Support for controlling leds on xbox 360 pad.

Hi!

> this patch is against current input tree.
>
> Xbox360 pad has four leds, which forms a circle.
> Unfortunately the leds itself are not independent, and
> we can't control them directle, but rather through
> sending commands which have predefined meaning (like
> turn first on, others off)
> This patch allows us to send these commands via leds
> subsystem. Commands itself are described here:
> http://www.free60.org/wiki/Gamepad.
>
> Led subsystem allows us to set brightness, but there is
> nothing like brightness on this device. So brightness is
> actually interpreted as the command (only values between
> 0 and 14 are accepted).

Ugh, no, I do not think we want to do that.

Devices with brightness 0 or 255 (off or on) are okay, but controlling
4 leds with one brightness is just wrong.

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2007-05-31 15:53:53

by Jan Kratochvil

[permalink] [raw]
Subject: Re: [PATCH] Support for controlling leds on xbox 360 pad.

Hi,

On Thu, 31 May 2007, Pavel Machek wrote:
>> Led subsystem allows us to set brightness, but there is
>> nothing like brightness on this device. So brightness is
>> actually interpreted as the command (only values between
>> 0 and 14 are accepted).
>
> Ugh, no, I do not think we want to do that.

finally! I was surprised that it took so long before someone made objection to
this.
I agree that interpreting brightness as command isn't nice. But what other
options do I have?
a) Do not use led subystem, and do everything on my own
b) Do not allow user to change state of leds on this device
c) Extend led subsystem to have something like command. But it is in
contradiction with desired simplicity of led sybsystem.
d) Control leds via force feedback effect
e) Make 4 leds available and use brightness 0 for off and 255 for on. But only
one of them can be active at one time. And I'll give up some flashing effects.

Well actually I don't like this options. Any suggestions? Thanks!

Jan

2007-05-31 16:10:22

by Richard Purdie

[permalink] [raw]
Subject: Re: [PATCH] Support for controlling leds on xbox 360 pad.

Hi,

On Thu, 2007-05-31 at 17:53 +0200, Jan Kratochvil wrote:
> On Thu, 31 May 2007, Pavel Machek wrote:
> >> Led subsystem allows us to set brightness, but there is
> >> nothing like brightness on this device. So brightness is
> >> actually interpreted as the command (only values between
> >> 0 and 14 are accepted).
> >
> > Ugh, no, I do not think we want to do that.
>
> finally! I was surprised that it took so long before someone made objection to
> this.

:)

This hadn't gone unnoticed by me and I didn't like it either at first
however I had a look at the hardware specs and your summary below is
pretty accurate...

> I agree that interpreting brightness as command isn't nice. But what other
> options do I have?
> a) Do not use led subystem, and do everything on my own
> b) Do not allow user to change state of leds on this device
> c) Extend led subsystem to have something like command. But it is in
> contradiction with desired simplicity of led sybsystem.

Since the commands are so device specific and no new device will be the
same, I doubt this is worth doing (or possible) in a generic way so
you're back to a).

> d) Control leds via force feedback effect
> e) Make 4 leds available and use brightness 0 for off and 255 for on. But only
> one of them can be active at one time. And I'll give up some flashing effects.

If you expose 4 leds, you break the API badly as only one can be active.
That is worse than using the brightness parameter IMO.

> Well actually I don't like this options. Any suggestions? Thanks!

After consideration, I agree with your approach of using the brightness
and I don't have a better suggestion. Its not ideal but its a good
compromise since it doesn't really impose any limitations on your use of
the hardware but doesn't need a custom implementation which is the
alternative. The mode is still a kind of brightness setting if you twist
the definition a bit ;-).

Cheers,

Richard


2007-06-03 18:03:16

by Jan Kratochvil

[permalink] [raw]
Subject: Re: [PATCH] Support for controlling leds on xbox 360 pad.

On Wed, 30 May 2007, Jan Kratochvil wrote:
> Hi,
>> In ideal world having the code in drivers/leds would be preferred but if
>> you want to add the LED code directly to the input driver in this case I
>> have no objection and it probably makes sense. The LED code is already
>> spread about a bit anyway...
>
> I wasn't happy about having this splited in two subsystems either. So I'll
> rewrite it to use led functionality directly from
> drivers/input/joystick/xpad.c.
>
Hello,
so I integrated this patch into xpad.c itself.

Xbox360 pad has four leds, which forms a circle. Unfortunately the leds
itself are not independent, and we can't control them directle, but
rather through sending commands which have predefined meaning (like turn
first on, others off) This patch allows us to send these commands via
leds subsystem. Commands itself are described here:
http://www.free60.org/wiki/Gamepad.

Led subsystem allows us to set brightness, but there is nothing like brightness
on this device. So brightness is actually interpreted as the command (only
values between 0 and 14 are accepted).

So this patch uses led subystem in such way that it makes led in
/sys/class/leds/xpad[0-9][0-9]/ for each attached xbox 360 pad.

Signed-off-by: Jan Kratochvil <[email protected]>
---
drivers/input/joystick/Kconfig | 7 ++
drivers/input/joystick/xpad.c | 134 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 137 insertions(+), 4 deletions(-)

diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index a15a923..2098ab6 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -274,4 +274,11 @@ config JOYSTICK_XPAD_FF
---help---
Say Y here if you want to take advantage of xbox 360 rumble features.

+config JOYSTICK_XPAD_LEDS
+ bool "LED Support for Xbox360 controller 'BigX' LED"
+ depends on LEDS_CLASS && JOYSTICK_XPAD_FF
+ ---help---
+ This option enables support for the LED which surrounds the Big X on
+ XBox 360 controller.
+
endif
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index f3f2ade..4a51d6f 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -198,6 +198,7 @@ struct usb_xpad {
struct urb *irq_out; /* urb for interrupt out report */
unsigned char *odata; /* output data */
dma_addr_t odata_dma;
+ struct mutex odata_mutex;
#endif

char phys[65]; /* physical device path */
@@ -418,6 +419,8 @@ static int xpad_init_ff(struct usb_inter
if (!xpad->odata)
goto fail1;

+ mutex_init(&xpad->odata_mutex);
+
xpad->irq_out = usb_alloc_urb(0, GFP_KERNEL);
if (!xpad->irq_out)
goto fail2;
@@ -463,6 +466,123 @@ static void xpad_stop_ff(struct usb_xpad
static void xpad_deinit_ff(struct usb_xpad *xpad) { }
#endif

+#if defined(CONFIG_JOYSTICK_XPAD_LEDS)
+#include <linux/leds.h>
+
+struct xpad_led {
+ struct led_classdev cdev;
+
+ unsigned int id;
+ struct usb_xpad * xpad;
+ struct list_head node;
+};
+
+static LIST_HEAD(xpad_led_list);
+
+static void xpad_send_command(int command, struct usb_xpad *xpad)
+{
+ if (command >= 0 && command < 14) {
+ mutex_lock(&xpad->odata_mutex);
+ xpad->odata[0] = 0x01;
+ xpad->odata[1] = 0x03;
+ xpad->odata[2] = command;
+ usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+ mutex_unlock(&xpad->odata_mutex);
+ }
+}
+
+static void xpad_led_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ struct xpad_led *led_dev =
+ container_of(led_cdev, struct xpad_led, cdev);
+
+ xpad_send_command(value, led_dev->xpad);
+}
+
+int xpad_led_probe(struct usb_xpad *xpad)
+{
+ int i = 0;
+ struct xpad_led *pos_led;
+ struct xpad_led *new_led;
+ int error = -ENOMEM;
+ char *name;
+
+ if (xpad->xtype != XTYPE_XBOX360)
+ return 0;
+
+ list_for_each_entry(pos_led, &xpad_led_list, node) {
+ if (pos_led->id == i)
+ i++;
+ else
+ break;
+ }
+
+ if (i > 99)
+ goto fail1;
+
+ new_led = kzalloc(sizeof(struct xpad_led), GFP_KERNEL);
+ if (!new_led)
+ goto fail1;
+
+ new_led->id = i;
+
+ name = kzalloc(sizeof(char)*7, GFP_KERNEL);
+ if (!name)
+ goto fail2;
+
+ strcpy(name, "xpad");
+ name[4] = '0' + i / 10;
+ name[5] = '0' + i % 10;
+ name[6] = 0;
+
+ new_led->cdev.name = name;
+ new_led->cdev.brightness_set = xpad_led_set;
+ new_led->xpad = xpad;
+
+ list_add_tail(&new_led->node, &pos_led->node);
+
+ if (led_classdev_register(&xpad->udev->dev, &new_led->cdev))
+ goto fail3;
+
+ xpad_send_command( (i % 4) + 2, xpad);
+
+ return 0;
+
+fail3:
+ kfree(new_led->cdev.name);
+fail2:
+ kfree(new_led);
+fail1:
+ return error;
+}
+
+int xpad_led_disconnect(struct usb_xpad *xpad)
+{
+ struct xpad_led *led;
+ struct list_head *pos, *tmp;
+
+ if (xpad->xtype != XTYPE_XBOX360)
+ return 0;
+
+ list_for_each_safe(pos, tmp, &xpad_led_list) {
+ led = list_entry(pos, struct xpad_led, node);
+ if (led->xpad == xpad) {
+ led_classdev_unregister(&led->cdev);
+ list_del(pos);
+ kfree(led->cdev.name);
+ kfree(led);
+ break;
+ }
+ }
+ return 0;
+}
+#else
+static int xpad_led_probe(struct usb_xpad *xpad) { return 0; }
+static int xpad_led_disconnect(struct usb_xpad *xpad) { return 0; }
+#endif
+
+
static int xpad_open(struct input_dev *dev)
{
struct usb_xpad *xpad = input_get_drvdata(dev);
@@ -575,6 +695,10 @@ static int xpad_probe(struct usb_interfa
if (error)
goto fail2;

+ error = xpad_led_probe(xpad);
+ if (error)
+ goto fail3;
+
ep_irq_in = &intf->cur_altsetting->endpoint[0].desc;
usb_fill_int_urb(xpad->irq_in, udev,
usb_rcvintpipe(udev, ep_irq_in->bEndpointAddress),
@@ -585,14 +709,15 @@ static int xpad_probe(struct usb_interfa

error = input_register_device(xpad->dev);
if (error)
- goto fail3;
+ goto fail4;

usb_set_intfdata(intf, xpad);
return 0;

- fail3: usb_free_urb(xpad->irq_in);
- fail2: usb_buffer_free(udev, XPAD_PKT_LEN, xpad->idata, xpad->idata_dma);
- fail1: input_free_device(input_dev);
+fail4: usb_free_urb(xpad->irq_in);
+fail3: xpad_deinit_ff(xpad);
+fail2: usb_buffer_free(udev, XPAD_PKT_LEN, xpad->idata, xpad->idata_dma);
+fail1: input_free_device(input_dev);
kfree(xpad);
return error;

@@ -604,6 +729,7 @@ static void xpad_disconnect(struct usb_i

usb_set_intfdata(intf, NULL);
if (xpad) {
+ xpad_led_disconnect(xpad);
input_unregister_device(xpad->dev);
xpad_deinit_ff(xpad);
usb_free_urb(xpad->irq_in);
--
1.4.3.4

2007-06-03 18:07:26

by Jan Kratochvil

[permalink] [raw]
Subject: Re: [PATCH] Support for controlling leds on xbox 360 pad.

Hi,
the previous patch made led support dependent on force feedback. Which
doesn't make sense from user point of view. So this patch removes this odd
dependency. I made it in separate patch to make my changes easier to
review.

Led support of xbox 360 now doesn't depend on force feedback.

Signed-off-by: Jan Kratochvil <[email protected]>
---
drivers/input/joystick/Kconfig | 2 +-
drivers/input/joystick/xpad.c | 87 ++++++++++++++++++++++------------------
2 files changed, 49 insertions(+), 40 deletions(-)

diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index 2098ab6..e6c7560 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -276,7 +276,7 @@ config JOYSTICK_XPAD_FF

config JOYSTICK_XPAD_LEDS
bool "LED Support for Xbox360 controller 'BigX' LED"
- depends on LEDS_CLASS && JOYSTICK_XPAD_FF
+ depends on LEDS_CLASS && JOYSTICK_XPAD
---help---
This option enables support for the LED which surrounds the Big X on
XBox 360 controller.
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 4a51d6f..b51229f 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -194,7 +194,7 @@ struct usb_xpad {
unsigned char *idata; /* input data */
dma_addr_t idata_dma;

-#ifdef CONFIG_JOYSTICK_XPAD_FF
+#if defined(CONFIG_JOYSTICK_XPAD_FF) || defined(CONFIG_JOYSTICK_XPAD_LEDS)
struct urb *irq_out; /* urb for interrupt out report */
unsigned char *odata; /* output data */
dma_addr_t odata_dma;
@@ -358,7 +358,7 @@ exit:
__FUNCTION__, retval);
}

-#ifdef CONFIG_JOYSTICK_XPAD_FF
+#if defined(CONFIG_JOYSTICK_XPAD_FF) || defined(CONFIG_JOYSTICK_XPAD_LEDS)
static void xpad_irq_out(struct urb *urb)
{
int retval;
@@ -385,28 +385,7 @@ exit:
__FUNCTION__, retval);
}

-int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
-{
- struct usb_xpad *xpad = input_get_drvdata(dev);
-
- if (effect->type == FF_RUMBLE) {
- __u16 strong = effect->u.rumble.strong_magnitude;
- __u16 weak = effect->u.rumble.weak_magnitude;
- xpad->odata[0] = 0x00;
- xpad->odata[1] = 0x08;
- xpad->odata[2] = 0x00;
- xpad->odata[3] = strong / 256;
- xpad->odata[4] = weak / 256;
- xpad->odata[5] = 0x00;
- xpad->odata[6] = 0x00;
- xpad->odata[7] = 0x00;
- usb_submit_urb(xpad->irq_out, GFP_KERNEL);
- }
-
- return 0;
-}
-
-static int xpad_init_ff(struct usb_interface *intf, struct usb_xpad *xpad)
+static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad)
{
struct usb_endpoint_descriptor *ep_irq_out;
int error = -ENOMEM;
@@ -433,25 +412,19 @@ static int xpad_init_ff(struct usb_inter
xpad->irq_out->transfer_dma = xpad->odata_dma;
xpad->irq_out->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;

- input_set_capability(xpad->dev, EV_FF, FF_RUMBLE);
-
- error = input_ff_create_memless(xpad->dev, NULL, xpad_play_effect);
- if (error)
- goto fail2;
-
return 0;

fail2: usb_buffer_free(xpad->udev, XPAD_PKT_LEN, xpad->odata, xpad->odata_dma);
fail1: return error;
}

-static void xpad_stop_ff(struct usb_xpad *xpad)
+static void xpad_stop_output(struct usb_xpad *xpad)
{
if (xpad->xtype == XTYPE_XBOX360)
usb_kill_urb(xpad->irq_out);
}

-static void xpad_deinit_ff(struct usb_xpad *xpad)
+static void xpad_deinit_output(struct usb_xpad *xpad)
{
if (xpad->xtype == XTYPE_XBOX360) {
usb_free_urb(xpad->irq_out);
@@ -459,11 +432,43 @@ static void xpad_deinit_ff(struct usb_xp
xpad->odata, xpad->odata_dma);
}
}
+#else
+static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad) { return 0; }
+static void xpad_deinit_output(struct usb_xpad *xpad) {}
+static void xpad_stop_output(struct usb_xpad *xpad) {}
+#endif
+
+#ifdef CONFIG_JOYSTICK_XPAD_FF
+int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
+{
+ struct usb_xpad *xpad = input_get_drvdata(dev);
+
+ if (effect->type == FF_RUMBLE) {
+ __u16 strong = effect->u.rumble.strong_magnitude;
+ __u16 weak = effect->u.rumble.weak_magnitude;
+ xpad->odata[0] = 0x00;
+ xpad->odata[1] = 0x08;
+ xpad->odata[2] = 0x00;
+ xpad->odata[3] = strong / 256;
+ xpad->odata[4] = weak / 256;
+ xpad->odata[5] = 0x00;
+ xpad->odata[6] = 0x00;
+ xpad->odata[7] = 0x00;
+ usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+ }
+
+ return 0;
+}
+
+static int xpad_init_ff(struct usb_xpad *xpad)
+{
+ input_set_capability(xpad->dev, EV_FF, FF_RUMBLE);
+
+ return input_ff_create_memless(xpad->dev, NULL, xpad_play_effect);
+}

#else
-static int xpad_init_ff(struct usb_interface *intf, struct usb_xpad *xpad) { return 0; }
-static void xpad_stop_ff(struct usb_xpad *xpad) { }
-static void xpad_deinit_ff(struct usb_xpad *xpad) { }
+static int xpad_init_ff(struct usb_xpad *xpad) { return 0; }
#endif

#if defined(CONFIG_JOYSTICK_XPAD_LEDS)
@@ -599,7 +604,7 @@ static void xpad_close(struct input_dev
struct usb_xpad *xpad = input_get_drvdata(dev);

usb_kill_urb(xpad->irq_in);
- xpad_stop_ff(xpad);
+ xpad_stop_output(xpad);
}

static void xpad_set_up_abs(struct input_dev *input_dev, signed short abs)
@@ -691,10 +696,14 @@ static int xpad_probe(struct usb_interfa
for (i = 0; xpad_abs_pad[i] >= 0; i++)
xpad_set_up_abs(input_dev, xpad_abs_pad[i]);

- error = xpad_init_ff(intf, xpad);
+ error = xpad_init_output(intf, xpad);
if (error)
goto fail2;

+ error = xpad_init_ff(xpad);
+ if (error)
+ goto fail3;
+
error = xpad_led_probe(xpad);
if (error)
goto fail3;
@@ -715,7 +724,7 @@ static int xpad_probe(struct usb_interfa
return 0;

fail4: usb_free_urb(xpad->irq_in);
-fail3: xpad_deinit_ff(xpad);
+fail3: xpad_deinit_output(xpad);
fail2: usb_buffer_free(udev, XPAD_PKT_LEN, xpad->idata, xpad->idata_dma);
fail1: input_free_device(input_dev);
kfree(xpad);
@@ -731,7 +740,7 @@ static void xpad_disconnect(struct usb_i
if (xpad) {
xpad_led_disconnect(xpad);
input_unregister_device(xpad->dev);
- xpad_deinit_ff(xpad);
+ xpad_deinit_output(xpad);
usb_free_urb(xpad->irq_in);
usb_buffer_free(xpad->udev, XPAD_PKT_LEN,
xpad->idata, xpad->idata_dma);
--
1.4.3.4



2007-06-15 04:15:43

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Support for controlling leds on xbox 360 pad.

Hi Jan,

On Sunday 03 June 2007 14:02, Jan Kratochvil wrote:
> +???????list_for_each_entry(pos_led, &xpad_led_list, node) {
> +???????????????if (pos_led->id == i)
> +???????????????????????i++;
> +???????????????else
> +???????????????????????break;
> +???????}

I don't like this list business, it requires locking and frankly
is not needed.

> +
> +???????if (i > 99)
> +???????????????goto fail1;

... and this limitation as well.

How about the patch below instead?

--
Dmitry

Subject: Input: xpad - add support for leds on xbox 360 pad
From: Jan Kratochvil <[email protected]>

Input: xpad - add support for leds on xbox 360 pad

Export LEDs on Xbox360 pad via led subsystem as a single device in
/sys/class/leds/xpad[0-9]+.

Xbox360 pad has four leds, which form a circle. Unfortunately the leds
can't be controlled independently and can only display a predefined
set of patterns (for example one is turned on wile others are off or
a rotating pattern - 1-2-3-4). To activate a pattern one needs to send
a specific command to the device (see http://www.free60.org/wiki/Gamepad).

Led subsystem allows us to set brightness, but there is nothing like
brightness on this device. So brightness is actually interpreted as
the command (only values between 0 and 14 are accepted).

Signed-off-by: Jan Kratochvil <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>
---

drivers/input/joystick/Kconfig | 7 +
drivers/input/joystick/xpad.c | 190 +++++++++++++++++++++++++++++++----------
2 files changed, 155 insertions(+), 42 deletions(-)

Index: work/drivers/input/joystick/Kconfig
===================================================================
--- work.orig/drivers/input/joystick/Kconfig
+++ work/drivers/input/joystick/Kconfig
@@ -275,4 +275,11 @@ config JOYSTICK_XPAD_FF
---help---
Say Y here if you want to take advantage of xbox 360 rumble features.

+config JOYSTICK_XPAD_LEDS
+ bool "LED Support for Xbox360 controller 'BigX' LED"
+ depends on LEDS_CLASS && JOYSTICK_XPAD
+ ---help---
+ This option enables support for the LED which surrounds the Big X on
+ XBox 360 controller.
+
endif
Index: work/drivers/input/joystick/xpad.c
===================================================================
--- work.orig/drivers/input/joystick/xpad.c
+++ work/drivers/input/joystick/xpad.c
@@ -191,13 +191,18 @@ struct usb_xpad {
unsigned char *idata; /* input data */
dma_addr_t idata_dma;

-#ifdef CONFIG_JOYSTICK_XPAD_FF
+#if defined(CONFIG_JOYSTICK_XPAD_FF) || defined(CONFIG_JOYSTICK_XPAD_LEDS)
struct urb *irq_out; /* urb for interrupt out report */
unsigned char *odata; /* output data */
dma_addr_t odata_dma;
+ struct mutex odata_mutex;
+#endif
+
+#if defined(CONFIG_JOYSTICK_XPAD_LEDS)
+ struct xpad_led *led;
#endif

- char phys[65]; /* physical device path */
+ char phys[64]; /* physical device path */

int dpad_mapping; /* map d-pad to buttons or to axes */
int xtype; /* type of xbox device */
@@ -349,7 +354,7 @@ exit:
__FUNCTION__, retval);
}

-#ifdef CONFIG_JOYSTICK_XPAD_FF
+#if defined(CONFIG_JOYSTICK_XPAD_FF) || defined(CONFIG_JOYSTICK_XPAD_LEDS)
static void xpad_irq_out(struct urb *urb)
{
int retval;
@@ -376,29 +381,7 @@ exit:
__FUNCTION__, retval);
}

-static int xpad_play_effect(struct input_dev *dev, void *data,
- struct ff_effect *effect)
-{
- struct usb_xpad *xpad = input_get_drvdata(dev);
-
- if (effect->type == FF_RUMBLE) {
- __u16 strong = effect->u.rumble.strong_magnitude;
- __u16 weak = effect->u.rumble.weak_magnitude;
- xpad->odata[0] = 0x00;
- xpad->odata[1] = 0x08;
- xpad->odata[2] = 0x00;
- xpad->odata[3] = strong / 256;
- xpad->odata[4] = weak / 256;
- xpad->odata[5] = 0x00;
- xpad->odata[6] = 0x00;
- xpad->odata[7] = 0x00;
- usb_submit_urb(xpad->irq_out, GFP_KERNEL);
- }
-
- return 0;
-}
-
-static int xpad_init_ff(struct usb_interface *intf, struct usb_xpad *xpad)
+static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad)
{
struct usb_endpoint_descriptor *ep_irq_out;
int error = -ENOMEM;
@@ -411,6 +394,8 @@ static int xpad_init_ff(struct usb_inter
if (!xpad->odata)
goto fail1;

+ mutex_init(&xpad->odata_mutex);
+
xpad->irq_out = usb_alloc_urb(0, GFP_KERNEL);
if (!xpad->irq_out)
goto fail2;
@@ -423,25 +408,19 @@ static int xpad_init_ff(struct usb_inter
xpad->irq_out->transfer_dma = xpad->odata_dma;
xpad->irq_out->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;

- input_set_capability(xpad->dev, EV_FF, FF_RUMBLE);
-
- error = input_ff_create_memless(xpad->dev, NULL, xpad_play_effect);
- if (error)
- goto fail2;
-
return 0;

fail2: usb_buffer_free(xpad->udev, XPAD_PKT_LEN, xpad->odata, xpad->odata_dma);
fail1: return error;
}

-static void xpad_stop_ff(struct usb_xpad *xpad)
+static void xpad_stop_output(struct usb_xpad *xpad)
{
if (xpad->xtype == XTYPE_XBOX360)
usb_kill_urb(xpad->irq_out);
}

-static void xpad_deinit_ff(struct usb_xpad *xpad)
+static void xpad_deinit_output(struct usb_xpad *xpad)
{
if (xpad->xtype == XTYPE_XBOX360) {
usb_free_urb(xpad->irq_out);
@@ -449,13 +428,130 @@ static void xpad_deinit_ff(struct usb_xp
xpad->odata, xpad->odata_dma);
}
}
+#else
+static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad) { return 0; }
+static void xpad_deinit_output(struct usb_xpad *xpad) {}
+static void xpad_stop_output(struct usb_xpad *xpad) {}
+#endif

+#ifdef CONFIG_JOYSTICK_XPAD_FF
+static int xpad_play_effect(struct input_dev *dev, void *data,
+ struct ff_effect *effect)
+{
+ struct usb_xpad *xpad = input_get_drvdata(dev);
+
+ if (effect->type == FF_RUMBLE) {
+ __u16 strong = effect->u.rumble.strong_magnitude;
+ __u16 weak = effect->u.rumble.weak_magnitude;
+ xpad->odata[0] = 0x00;
+ xpad->odata[1] = 0x08;
+ xpad->odata[2] = 0x00;
+ xpad->odata[3] = strong / 256;
+ xpad->odata[4] = weak / 256;
+ xpad->odata[5] = 0x00;
+ xpad->odata[6] = 0x00;
+ xpad->odata[7] = 0x00;
+ usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+ }
+
+ return 0;
+}
+
+static int xpad_init_ff(struct usb_xpad *xpad)
+{
+ input_set_capability(xpad->dev, EV_FF, FF_RUMBLE);
+
+ return input_ff_create_memless(xpad->dev, NULL, xpad_play_effect);
+}
+
+#else
+static int xpad_init_ff(struct usb_xpad *xpad) { return 0; }
+#endif
+
+#if defined(CONFIG_JOYSTICK_XPAD_LEDS)
+#include <linux/leds.h>
+
+struct xpad_led {
+ char name[16];
+ struct led_classdev led_cdev;
+ struct usb_xpad *xpad;
+};
+
+static void xpad_send_led_command(struct usb_xpad *xpad, int command)
+{
+ if (command >= 0 && command < 14) {
+ mutex_lock(&xpad->odata_mutex);
+ xpad->odata[0] = 0x01;
+ xpad->odata[1] = 0x03;
+ xpad->odata[2] = command;
+ usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+ mutex_unlock(&xpad->odata_mutex);
+ }
+}
+
+static void xpad_led_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ struct xpad_led *xpad_led = container_of(led_cdev,
+ struct xpad_led, led_cdev);
+
+ xpad_send_led_command(xpad_led->xpad, value);
+}
+
+static int xpad_led_probe(struct usb_xpad *xpad)
+{
+ static atomic_t led_seq = ATOMIC_INIT(0);
+ long led_no;
+ struct xpad_led *led;
+ struct led_classdev *led_cdev;
+ int error;
+
+ if (xpad->xtype != XTYPE_XBOX360)
+ return 0;
+
+ xpad->led = led = kzalloc(sizeof(struct xpad_led), GFP_KERNEL);
+ if (!led)
+ return -ENOMEM;
+
+ led_no = (long)atomic_inc_return(&led_no) - 1;
+
+ snprintf(led->name, sizeof(led->name), "xpad%ld", led_no);
+ led->xpad = xpad;
+
+ led_cdev = &led->led_cdev;
+ led_cdev->name = led->name;
+ led_cdev->brightness_set = xpad_led_set;
+
+ error = led_classdev_register(&xpad->udev->dev, led_cdev);
+ if (error) {
+ kfree(led);
+ xpad->led = NULL;
+ return error;
+ }
+
+ /*
+ * Light up the segment corresponding to controller number
+ */
+ xpad_send_led_command(xpad, (led_no % 4) + 2);
+
+ return 0;
+}
+
+static void xpad_led_disconnect(struct usb_xpad *xpad)
+{
+ struct xpad_led *xpad_led = xpad->led;
+
+ if (xpad_led) {
+ led_classdev_unregister(&xpad_led->led_cdev);
+ kfree(xpad_led->name);
+ }
+}
#else
-static int xpad_init_ff(struct usb_interface *intf, struct usb_xpad *xpad) { return 0; }
-static void xpad_stop_ff(struct usb_xpad *xpad) { }
-static void xpad_deinit_ff(struct usb_xpad *xpad) { }
+static int xpad_led_probe(struct usb_xpad *xpad) { return 0; }
+static void xpad_led_disconnect(struct usb_xpad *xpad) { }
#endif

+
static int xpad_open(struct input_dev *dev)
{
struct usb_xpad *xpad = input_get_drvdata(dev);
@@ -472,7 +568,7 @@ static void xpad_close(struct input_dev
struct usb_xpad *xpad = input_get_drvdata(dev);

usb_kill_urb(xpad->irq_in);
- xpad_stop_ff(xpad);
+ xpad_stop_output(xpad);
}

static void xpad_set_up_abs(struct input_dev *input_dev, signed short abs)
@@ -564,10 +660,18 @@ static int xpad_probe(struct usb_interfa
for (i = 0; xpad_abs_pad[i] >= 0; i++)
xpad_set_up_abs(input_dev, xpad_abs_pad[i]);

- error = xpad_init_ff(intf, xpad);
+ error = xpad_init_output(intf, xpad);
if (error)
goto fail2;

+ error = xpad_init_ff(xpad);
+ if (error)
+ goto fail3;
+
+ error = xpad_led_probe(xpad);
+ if (error)
+ goto fail3;
+
ep_irq_in = &intf->cur_altsetting->endpoint[0].desc;
usb_fill_int_urb(xpad->irq_in, udev,
usb_rcvintpipe(udev, ep_irq_in->bEndpointAddress),
@@ -578,12 +682,13 @@ static int xpad_probe(struct usb_interfa

error = input_register_device(xpad->dev);
if (error)
- goto fail3;
+ goto fail4;

usb_set_intfdata(intf, xpad);
return 0;

- fail3: usb_free_urb(xpad->irq_in);
+ fail4: usb_free_urb(xpad->irq_in);
+ fail3: xpad_deinit_output(xpad);
fail2: usb_buffer_free(udev, XPAD_PKT_LEN, xpad->idata, xpad->idata_dma);
fail1: input_free_device(input_dev);
kfree(xpad);
@@ -597,8 +702,9 @@ static void xpad_disconnect(struct usb_i

usb_set_intfdata(intf, NULL);
if (xpad) {
+ xpad_led_disconnect(xpad);
input_unregister_device(xpad->dev);
- xpad_deinit_ff(xpad);
+ xpad_deinit_output(xpad);
usb_free_urb(xpad->irq_in);
usb_buffer_free(xpad->udev, XPAD_PKT_LEN,
xpad->idata, xpad->idata_dma);

2007-06-15 05:06:58

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Support for controlling leds on xbox 360 pad.

On Friday 15 June 2007 00:15, Dmitry Torokhov wrote:
> How about the patch below instead?
>

And one more time without warnings...

--
Dmitry

Subject: Input: xpad - add support for leds on xbox 360 pad
From: Jan Kratochvil <[email protected]>

Input: xpad - add support for leds on xbox 360 pad

Export LEDs on Xbox360 pad via led subsystem as a single device in
/sys/class/leds/xpad[0-9]+.

Xbox360 pad has four leds, which form a circle. Unfortunately the leds
can't be controlled independently and can only display a predefined
set of patterns (for example one is turned on wile others are off or
a rotating pattern - 1-2-3-4). To activate a pattern one needs to send
a specific command to the device (see http://www.free60.org/wiki/Gamepad).

Led subsystem allows us to set brightness, but there is nothing like
brightness on this device. So brightness is actually interpreted as
the command (only values between 0 and 14 are accepted).

Signed-off-by: Jan Kratochvil <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>
---

drivers/input/joystick/Kconfig | 7 +
drivers/input/joystick/xpad.c | 190 +++++++++++++++++++++++++++++++----------
2 files changed, 155 insertions(+), 42 deletions(-)

Index: work/drivers/input/joystick/Kconfig
===================================================================
--- work.orig/drivers/input/joystick/Kconfig
+++ work/drivers/input/joystick/Kconfig
@@ -275,4 +275,11 @@ config JOYSTICK_XPAD_FF
---help---
Say Y here if you want to take advantage of xbox 360 rumble features.

+config JOYSTICK_XPAD_LEDS
+ bool "LED Support for Xbox360 controller 'BigX' LED"
+ depends on LEDS_CLASS && JOYSTICK_XPAD
+ ---help---
+ This option enables support for the LED which surrounds the Big X on
+ XBox 360 controller.
+
endif
Index: work/drivers/input/joystick/xpad.c
===================================================================
--- work.orig/drivers/input/joystick/xpad.c
+++ work/drivers/input/joystick/xpad.c
@@ -191,13 +191,18 @@ struct usb_xpad {
unsigned char *idata; /* input data */
dma_addr_t idata_dma;

-#ifdef CONFIG_JOYSTICK_XPAD_FF
+#if defined(CONFIG_JOYSTICK_XPAD_FF) || defined(CONFIG_JOYSTICK_XPAD_LEDS)
struct urb *irq_out; /* urb for interrupt out report */
unsigned char *odata; /* output data */
dma_addr_t odata_dma;
+ struct mutex odata_mutex;
+#endif
+
+#if defined(CONFIG_JOYSTICK_XPAD_LEDS)
+ struct xpad_led *led;
#endif

- char phys[65]; /* physical device path */
+ char phys[64]; /* physical device path */

int dpad_mapping; /* map d-pad to buttons or to axes */
int xtype; /* type of xbox device */
@@ -349,7 +354,7 @@ exit:
__FUNCTION__, retval);
}

-#ifdef CONFIG_JOYSTICK_XPAD_FF
+#if defined(CONFIG_JOYSTICK_XPAD_FF) || defined(CONFIG_JOYSTICK_XPAD_LEDS)
static void xpad_irq_out(struct urb *urb)
{
int retval;
@@ -376,29 +381,7 @@ exit:
__FUNCTION__, retval);
}

-static int xpad_play_effect(struct input_dev *dev, void *data,
- struct ff_effect *effect)
-{
- struct usb_xpad *xpad = input_get_drvdata(dev);
-
- if (effect->type == FF_RUMBLE) {
- __u16 strong = effect->u.rumble.strong_magnitude;
- __u16 weak = effect->u.rumble.weak_magnitude;
- xpad->odata[0] = 0x00;
- xpad->odata[1] = 0x08;
- xpad->odata[2] = 0x00;
- xpad->odata[3] = strong / 256;
- xpad->odata[4] = weak / 256;
- xpad->odata[5] = 0x00;
- xpad->odata[6] = 0x00;
- xpad->odata[7] = 0x00;
- usb_submit_urb(xpad->irq_out, GFP_KERNEL);
- }
-
- return 0;
-}
-
-static int xpad_init_ff(struct usb_interface *intf, struct usb_xpad *xpad)
+static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad)
{
struct usb_endpoint_descriptor *ep_irq_out;
int error = -ENOMEM;
@@ -411,6 +394,8 @@ static int xpad_init_ff(struct usb_inter
if (!xpad->odata)
goto fail1;

+ mutex_init(&xpad->odata_mutex);
+
xpad->irq_out = usb_alloc_urb(0, GFP_KERNEL);
if (!xpad->irq_out)
goto fail2;
@@ -423,25 +408,19 @@ static int xpad_init_ff(struct usb_inter
xpad->irq_out->transfer_dma = xpad->odata_dma;
xpad->irq_out->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;

- input_set_capability(xpad->dev, EV_FF, FF_RUMBLE);
-
- error = input_ff_create_memless(xpad->dev, NULL, xpad_play_effect);
- if (error)
- goto fail2;
-
return 0;

fail2: usb_buffer_free(xpad->udev, XPAD_PKT_LEN, xpad->odata, xpad->odata_dma);
fail1: return error;
}

-static void xpad_stop_ff(struct usb_xpad *xpad)
+static void xpad_stop_output(struct usb_xpad *xpad)
{
if (xpad->xtype == XTYPE_XBOX360)
usb_kill_urb(xpad->irq_out);
}

-static void xpad_deinit_ff(struct usb_xpad *xpad)
+static void xpad_deinit_output(struct usb_xpad *xpad)
{
if (xpad->xtype == XTYPE_XBOX360) {
usb_free_urb(xpad->irq_out);
@@ -449,13 +428,130 @@ static void xpad_deinit_ff(struct usb_xp
xpad->odata, xpad->odata_dma);
}
}
+#else
+static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad) { return 0; }
+static void xpad_deinit_output(struct usb_xpad *xpad) {}
+static void xpad_stop_output(struct usb_xpad *xpad) {}
+#endif

+#ifdef CONFIG_JOYSTICK_XPAD_FF
+static int xpad_play_effect(struct input_dev *dev, void *data,
+ struct ff_effect *effect)
+{
+ struct usb_xpad *xpad = input_get_drvdata(dev);
+
+ if (effect->type == FF_RUMBLE) {
+ __u16 strong = effect->u.rumble.strong_magnitude;
+ __u16 weak = effect->u.rumble.weak_magnitude;
+ xpad->odata[0] = 0x00;
+ xpad->odata[1] = 0x08;
+ xpad->odata[2] = 0x00;
+ xpad->odata[3] = strong / 256;
+ xpad->odata[4] = weak / 256;
+ xpad->odata[5] = 0x00;
+ xpad->odata[6] = 0x00;
+ xpad->odata[7] = 0x00;
+ usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+ }
+
+ return 0;
+}
+
+static int xpad_init_ff(struct usb_xpad *xpad)
+{
+ input_set_capability(xpad->dev, EV_FF, FF_RUMBLE);
+
+ return input_ff_create_memless(xpad->dev, NULL, xpad_play_effect);
+}
+
+#else
+static int xpad_init_ff(struct usb_xpad *xpad) { return 0; }
+#endif
+
+#if defined(CONFIG_JOYSTICK_XPAD_LEDS)
+#include <linux/leds.h>
+
+struct xpad_led {
+ char name[16];
+ struct led_classdev led_cdev;
+ struct usb_xpad *xpad;
+};
+
+static void xpad_send_led_command(struct usb_xpad *xpad, int command)
+{
+ if (command >= 0 && command < 14) {
+ mutex_lock(&xpad->odata_mutex);
+ xpad->odata[0] = 0x01;
+ xpad->odata[1] = 0x03;
+ xpad->odata[2] = command;
+ usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+ mutex_unlock(&xpad->odata_mutex);
+ }
+}
+
+static void xpad_led_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ struct xpad_led *xpad_led = container_of(led_cdev,
+ struct xpad_led, led_cdev);
+
+ xpad_send_led_command(xpad_led->xpad, value);
+}
+
+static int xpad_led_probe(struct usb_xpad *xpad)
+{
+ static atomic_t led_seq = ATOMIC_INIT(0);
+ long led_no;
+ struct xpad_led *led;
+ struct led_classdev *led_cdev;
+ int error;
+
+ if (xpad->xtype != XTYPE_XBOX360)
+ return 0;
+
+ xpad->led = led = kzalloc(sizeof(struct xpad_led), GFP_KERNEL);
+ if (!led)
+ return -ENOMEM;
+
+ led_no = (long)atomic_inc_return(&led_seq) - 1;
+
+ snprintf(led->name, sizeof(led->name), "xpad%ld", led_no);
+ led->xpad = xpad;
+
+ led_cdev = &led->led_cdev;
+ led_cdev->name = led->name;
+ led_cdev->brightness_set = xpad_led_set;
+
+ error = led_classdev_register(&xpad->udev->dev, led_cdev);
+ if (error) {
+ kfree(led);
+ xpad->led = NULL;
+ return error;
+ }
+
+ /*
+ * Light up the segment corresponding to controller number
+ */
+ xpad_send_led_command(xpad, (led_no % 4) + 2);
+
+ return 0;
+}
+
+static void xpad_led_disconnect(struct usb_xpad *xpad)
+{
+ struct xpad_led *xpad_led = xpad->led;
+
+ if (xpad_led) {
+ led_classdev_unregister(&xpad_led->led_cdev);
+ kfree(xpad_led->name);
+ }
+}
#else
-static int xpad_init_ff(struct usb_interface *intf, struct usb_xpad *xpad) { return 0; }
-static void xpad_stop_ff(struct usb_xpad *xpad) { }
-static void xpad_deinit_ff(struct usb_xpad *xpad) { }
+static int xpad_led_probe(struct usb_xpad *xpad) { return 0; }
+static void xpad_led_disconnect(struct usb_xpad *xpad) { }
#endif

+
static int xpad_open(struct input_dev *dev)
{
struct usb_xpad *xpad = input_get_drvdata(dev);
@@ -472,7 +568,7 @@ static void xpad_close(struct input_dev
struct usb_xpad *xpad = input_get_drvdata(dev);

usb_kill_urb(xpad->irq_in);
- xpad_stop_ff(xpad);
+ xpad_stop_output(xpad);
}

static void xpad_set_up_abs(struct input_dev *input_dev, signed short abs)
@@ -564,10 +660,18 @@ static int xpad_probe(struct usb_interfa
for (i = 0; xpad_abs_pad[i] >= 0; i++)
xpad_set_up_abs(input_dev, xpad_abs_pad[i]);

- error = xpad_init_ff(intf, xpad);
+ error = xpad_init_output(intf, xpad);
if (error)
goto fail2;

+ error = xpad_init_ff(xpad);
+ if (error)
+ goto fail3;
+
+ error = xpad_led_probe(xpad);
+ if (error)
+ goto fail3;
+
ep_irq_in = &intf->cur_altsetting->endpoint[0].desc;
usb_fill_int_urb(xpad->irq_in, udev,
usb_rcvintpipe(udev, ep_irq_in->bEndpointAddress),
@@ -578,12 +682,13 @@ static int xpad_probe(struct usb_interfa

error = input_register_device(xpad->dev);
if (error)
- goto fail3;
+ goto fail4;

usb_set_intfdata(intf, xpad);
return 0;

- fail3: usb_free_urb(xpad->irq_in);
+ fail4: usb_free_urb(xpad->irq_in);
+ fail3: xpad_deinit_output(xpad);
fail2: usb_buffer_free(udev, XPAD_PKT_LEN, xpad->idata, xpad->idata_dma);
fail1: input_free_device(input_dev);
kfree(xpad);
@@ -597,8 +702,9 @@ static void xpad_disconnect(struct usb_i

usb_set_intfdata(intf, NULL);
if (xpad) {
+ xpad_led_disconnect(xpad);
input_unregister_device(xpad->dev);
- xpad_deinit_ff(xpad);
+ xpad_deinit_output(xpad);
usb_free_urb(xpad->irq_in);
usb_buffer_free(xpad->udev, XPAD_PKT_LEN,
xpad->idata, xpad->idata_dma);