* Re: Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: Sven Eckelmann @ 2013-11-17 23:53 UTC (permalink / raw)
To: Antonio Ospite; +Cc: simon, linux-input, Jiri Kosina, Colin Leitner
In-Reply-To: <5434362.OvDlhOjSJd@sven-edge>
[-- Attachment #1: Type: text/plain, Size: 1054 bytes --]
On Monday 18 November 2013 00:12:14 Sven Eckelmann wrote:
> > Also please make sure that setting the rumble does not change the LEDs
> > status if there is any set: HID output report 1 is used for both LEDs
> > and rumble. In the bluez plugin[1] I plan on setting LEDs from userspace
> > to match the joystick number, just as the PS3 does, it would be strange
> > for the user if a rumble event would reset the LEDs status.
>
> I never used the LEDs and therefore cannot say anything about it (I don't
> have a specification for the used command format). Maybe I can try to play
> with them next week.
>
> But you're patch has some comments in set_leds. Do I correctly interpret the
> byte 10 in leds_report as "only make changes to following LEDs"? So setting
> it to 1 would make the command not change the LEDs at all?
Ok, just tried it and it seems this byte is really for the LEDs. But
unfortunately, it is enabling/disabling the LEDs completely and not the
configuration.
And sending less bytes just lets everything fail.
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: Re: Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: Sven Eckelmann @ 2013-11-18 0:26 UTC (permalink / raw)
To: Antonio Ospite; +Cc: simon, linux-input, Jiri Kosina, Colin Leitner
In-Reply-To: <8249378.0C4gn7PafG@sven-desktop>
[-- Attachment #1.1: Type: text/plain, Size: 543 bytes --]
On Monday 18 November 2013 00:53:25 Sven Eckelmann wrote:
[..]
> Ok, just tried it and it seems this byte is really for the LEDs. But
> unfortunately, it is enabling/disabling the LEDs completely and not the
> configuration.
>
> And sending less bytes just lets everything fail.
Forgot to add my proof of concept patch for the LED control. I've attached it
so you can also try it out.
@Simon: This may also be interesting for you because you've asked for it in
8c40cb08b7c44f373c2c533614d70b6a.squirrel@mungewell.org
Kind regards,
Sven
[-- Attachment #1.2: 0001-HID-sony-Make-sixaxis-usb-LEDs-configurable.patch --]
[-- Type: text/x-patch, Size: 9970 bytes --]
>From 94d33d53718268d3f41f9ec38105e221e0ba3585 Mon Sep 17 00:00:00 2001
From: Sven Eckelmann <sven@narfation.org>
Date: Mon, 18 Nov 2013 01:22:39 +0100
Subject: [RFC] HID: sony: Make sixaxis usb LEDs configurable
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
drivers/hid/hid-sony.c | 144 +++++++++++++++++++++++++------------------------
1 file changed, 74 insertions(+), 70 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 098af2f8..d1d99bb 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -224,18 +224,13 @@ static const unsigned int buzz_keymap[] = {
struct sony_sc {
unsigned long quirks;
+ struct work_struct state_worker;
+ struct hid_device *hdev;
#ifdef CONFIG_SONY_FF
- struct work_struct rumble_worker;
- struct hid_device *hdev;
__u8 left;
__u8 right;
#endif
-
- void *extra;
-};
-
-struct buzz_extra {
int led_state;
struct led_classdev *leds[4];
};
@@ -466,58 +461,66 @@ static void buzz_set_leds(struct hid_device *hdev, int leds)
hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
}
-static void buzz_led_set_brightness(struct led_classdev *led,
+static void sony_set_leds(struct hid_device *hdev, int leds)
+{
+ struct sony_sc *drv_data = hid_get_drvdata(hdev);
+
+ if (drv_data->quirks & BUZZ_CONTROLLER) {
+ buzz_set_leds(hdev, leds);
+ } else if (drv_data->quirks & SIXAXIS_CONTROLLER_USB) {
+ drv_data->led_state = leds;
+ schedule_work(&drv_data->state_worker);
+ }
+}
+
+static void sony_led_set_brightness(struct led_classdev *led,
enum led_brightness value)
{
struct device *dev = led->dev->parent;
struct hid_device *hdev = container_of(dev, struct hid_device, dev);
struct sony_sc *drv_data;
- struct buzz_extra *buzz;
int n;
drv_data = hid_get_drvdata(hdev);
- if (!drv_data || !drv_data->extra) {
+ if (!drv_data) {
hid_err(hdev, "No device data\n");
return;
}
- buzz = drv_data->extra;
for (n = 0; n < 4; n++) {
- if (led == buzz->leds[n]) {
- int on = !! (buzz->led_state & (1 << n));
+ if (led == drv_data->leds[n]) {
+ int on = !! (drv_data->led_state & (1 << n));
if (value == LED_OFF && on) {
- buzz->led_state &= ~(1 << n);
- buzz_set_leds(hdev, buzz->led_state);
+ drv_data->led_state &= ~(1 << n);
+ sony_set_leds(hdev, drv_data->led_state);
} else if (value != LED_OFF && !on) {
- buzz->led_state |= (1 << n);
- buzz_set_leds(hdev, buzz->led_state);
+ drv_data->led_state |= (1 << n);
+ sony_set_leds(hdev, drv_data->led_state);
}
break;
}
}
}
-static enum led_brightness buzz_led_get_brightness(struct led_classdev *led)
+static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
{
struct device *dev = led->dev->parent;
struct hid_device *hdev = container_of(dev, struct hid_device, dev);
struct sony_sc *drv_data;
- struct buzz_extra *buzz;
int n;
int on = 0;
drv_data = hid_get_drvdata(hdev);
- if (!drv_data || !drv_data->extra) {
+ if (!drv_data) {
hid_err(hdev, "No device data\n");
return LED_OFF;
}
- buzz = drv_data->extra;
for (n = 0; n < 4; n++) {
- if (led == buzz->leds[n]) {
- on = !! (buzz->led_state & (1 << n));
+ if (led == drv_data->leds[n]) {
+ on = !! (drv_data->led_state & (1 << n));
break;
}
}
@@ -525,35 +528,36 @@ static enum led_brightness buzz_led_get_brightness(struct led_classdev *led)
return on ? LED_FULL : LED_OFF;
}
-static int buzz_init(struct hid_device *hdev)
+static int sony_leds_init(struct hid_device *hdev)
{
struct sony_sc *drv_data;
- struct buzz_extra *buzz;
int n, ret = 0;
struct led_classdev *led;
size_t name_sz;
char *name;
+ size_t name_len;
+ const char *name_format;
drv_data = hid_get_drvdata(hdev);
- BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
+ BUG_ON(!(drv_data->quirks & (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER)));
- /* Validate expected report characteristics. */
- if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
- return -ENODEV;
-
- buzz = kzalloc(sizeof(*buzz), GFP_KERNEL);
- if (!buzz) {
- hid_err(hdev, "Insufficient memory, cannot allocate driver data\n");
- return -ENOMEM;
+ if (drv_data->quirks & BUZZ_CONTROLLER) {
+ name_len = strlen("::buzz#");
+ name_format = "%s::buzz%d";
+ /* Validate expected report characteristics. */
+ if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
+ return -ENODEV;
+ } else {
+ name_len = strlen("::sony#");
+ name_format = "%s::sony%d";
}
- drv_data->extra = buzz;
/* Clear LEDs as we have no way of reading their initial state. This is
* only relevant if the driver is loaded after somebody actively set the
* LEDs to on */
- buzz_set_leds(hdev, 0x00);
+ sony_set_leds(hdev, 0x00);
- name_sz = strlen(dev_name(&hdev->dev)) + strlen("::buzz#") + 1;
+ name_sz = strlen(dev_name(&hdev->dev)) + name_len + 1;
for (n = 0; n < 4; n++) {
led = kzalloc(sizeof(struct led_classdev) + name_sz, GFP_KERNEL);
@@ -563,12 +567,12 @@ static int buzz_init(struct hid_device *hdev)
}
name = (void *)(&led[1]);
- snprintf(name, name_sz, "%s::buzz%d", dev_name(&hdev->dev), n + 1);
+ snprintf(name, name_sz, name_format, dev_name(&hdev->dev), n + 1);
led->name = name;
led->brightness = 0;
led->max_brightness = 1;
- led->brightness_get = buzz_led_get_brightness;
- led->brightness_set = buzz_led_set_brightness;
+ led->brightness_get = sony_led_get_brightness;
+ led->brightness_set = sony_led_set_brightness;
if (led_classdev_register(&hdev->dev, led)) {
hid_err(hdev, "Failed to register LED %d\n", n);
@@ -576,73 +580,71 @@ static int buzz_init(struct hid_device *hdev)
goto error_leds;
}
- buzz->leds[n] = led;
+ drv_data->leds[n] = led;
}
return ret;
error_leds:
for (n = 0; n < 4; n++) {
- led = buzz->leds[n];
- buzz->leds[n] = NULL;
+ led = drv_data->leds[n];
+ drv_data->leds[n] = NULL;
if (!led)
continue;
led_classdev_unregister(led);
kfree(led);
}
- kfree(drv_data->extra);
- drv_data->extra = NULL;
return ret;
}
-static void buzz_remove(struct hid_device *hdev)
+static void sony_leds_remove(struct hid_device *hdev)
{
struct sony_sc *drv_data;
- struct buzz_extra *buzz;
struct led_classdev *led;
int n;
drv_data = hid_get_drvdata(hdev);
- BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
+ BUG_ON(!(drv_data->quirks & (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER)));
- buzz = drv_data->extra;
for (n = 0; n < 4; n++) {
- led = buzz->leds[n];
- buzz->leds[n] = NULL;
+ led = drv_data->leds[n];
+ drv_data->leds[n] = NULL;
if (!led)
continue;
led_classdev_unregister(led);
kfree(led);
}
-
- kfree(drv_data->extra);
- drv_data->extra = NULL;
}
-#ifdef CONFIG_SONY_FF
-static void sony_rumble_worker(struct work_struct *work)
+static void sony_state_worker(struct work_struct *work)
{
- struct sony_sc *sc = container_of(work, struct sony_sc, rumble_worker);
+ struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
unsigned char buf[] = {
0x01,
- 0x00, 0xff, 0x00, 0xff, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x03,
+ 0x00, 0xff, 0x00, 0xf0, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00,
0xff, 0x27, 0x10, 0x00, 0x32,
0xff, 0x27, 0x10, 0x00, 0x32,
0xff, 0x27, 0x10, 0x00, 0x32,
0xff, 0x27, 0x10, 0x00, 0x32,
0x00, 0x00, 0x00, 0x00, 0x00
};
+ size_t len = sizeof(buf);
+#ifdef CONFIG_SONY_FF
buf[3] = sc->right;
buf[5] = sc->left;
+#endif
- sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf),
+ buf[10] |= (sc->led_state & 0xf) << 1;
+
+ sc->hdev->hid_output_raw_report(sc->hdev, buf, len,
HID_OUTPUT_REPORT);
}
+#ifdef CONFIG_SONY_FF
static int sony_play_effect(struct input_dev *dev, void *data,
struct ff_effect *effect)
{
@@ -655,7 +657,7 @@ static int sony_play_effect(struct input_dev *dev, void *data,
sc->left = effect->u.rumble.strong_magnitude / 256;
sc->right = effect->u.rumble.weak_magnitude ? 1 : 0;
- schedule_work(&sc->rumble_worker);
+ schedule_work(&sc->state_worker);
return 0;
}
@@ -664,10 +666,6 @@ static int sony_init_ff(struct hid_device *hdev)
struct hid_input *hidinput = list_entry(hdev->inputs.next,
struct hid_input, list);
struct input_dev *input_dev = hidinput->input;
- struct sony_sc *sc = hid_get_drvdata(hdev);
-
- sc->hdev = hdev;
- INIT_WORK(&sc->rumble_worker, sony_rumble_worker);
input_set_capability(input_dev, EV_FF, FF_RUMBLE);
return input_ff_create_memless(input_dev, NULL, sony_play_effect);
@@ -677,7 +675,7 @@ static void sony_destroy_ff(struct hid_device *hdev)
{
struct sony_sc *sc = hid_get_drvdata(hdev);
- cancel_work_sync(&sc->rumble_worker);
+ cancel_work_sync(&sc->state_worker);
}
#else
@@ -706,6 +704,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
sc->quirks = quirks;
hid_set_drvdata(hdev, sc);
+ sc->hdev = hdev;
ret = hid_parse(hdev);
if (ret) {
@@ -729,17 +728,22 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
hdev->hid_output_raw_report = sixaxis_usb_output_raw_report;
ret = sixaxis_set_operational_usb(hdev);
+ INIT_WORK(&sc->state_worker, sony_state_worker);
}
else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
ret = sixaxis_set_operational_bt(hdev);
- else if (sc->quirks & BUZZ_CONTROLLER)
- ret = buzz_init(hdev);
else
ret = 0;
if (ret < 0)
goto err_stop;
+ if (sc->quirks & (SIXAXIS_CONTROLLER_USB | SIXAXIS_CONTROLLER_USB)) {
+ ret = sony_leds_init(hdev);
+ if (ret < 0)
+ goto err_stop;
+ }
+
ret = sony_init_ff(hdev);
if (ret < 0)
goto err_stop;
@@ -754,8 +758,8 @@ static void sony_remove(struct hid_device *hdev)
{
struct sony_sc *sc = hid_get_drvdata(hdev);
- if (sc->quirks & BUZZ_CONTROLLER)
- buzz_remove(hdev);
+ if (sc->quirks & (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER))
+ sony_leds_remove(hdev);
sony_destroy_ff(hdev);
--
1.8.4.3
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply related
* Re: Re: Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: simon @ 2013-11-18 1:21 UTC (permalink / raw)
To: Sven Eckelmann
Cc: Antonio Ospite, simon, linux-input, Jiri Kosina, Colin Leitner
In-Reply-To: <2105006.tGLFZqe9L3@sven-desktop>
> @Simon: This may also be interesting for you because you've asked for it
> in 8c40cb08b7c44f373c2c533614d70b6a.squirrel@mungewell.org
Yes, very interesting. Builds on top of the other 2 patches.
This seems to work, however something is causing the LEDs to go out once
they have been set. This seems to happen a few seconds after they are set,
but there does not appear too be USB activity (via usbmon).
If 'testrumble' is used then the previously set LEDs come back on.
I note that the LED class directory is based on USB ID and node, which
changes each time the controller is plugged in. This makes assigning LEDs
in game configurations a pain.
>From my point of view a 'fixed UID' (such as a serial number) would be
better, but most controllers don't report these. Any suggestions?
Simon
^ permalink raw reply
* Re: Re: Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: simon @ 2013-11-18 3:54 UTC (permalink / raw)
Cc: Sven Eckelmann, Antonio Ospite, simon, linux-input, Jiri Kosina,
Colin Leitner
In-Reply-To: <0d5dd16c7b4799f333fd4b4077e6c75c.squirrel@mungewell.org>
[-- Attachment #1: Type: text/plain, Size: 1846 bytes --]
> This seems to work, however something is causing the LEDs to go out once
> they have been set. This seems to happen a few seconds after they are set,
> but there does not appear too be USB activity (via usbmon).
I tested this on a real machine (as opposed to VM box), and the LEDs
remain set and do not auto cancel. Suspect host OS was doing something to
device.
There is another bit of weirdness though, when plugged in my Intec
controller slow flashes the LEDs. This is overwritten when I set any LED
through the LED class, but returns to flashing when I clear all LEDs via
the class.
IE. there is no way to turn all the the LEDs off.
The DualShock3/SixAxis behaves slightly differently in that the flashing
does not return when all the LEDs are turn off.
>From some of my earlier investigations I had some notes about LED settings
which might be of help.
--
# \x01 Header
# \x00\x00\x00\x00\x00 ForceFeedback
(\x00\xfe\x00+<rumble,40=weak,80=strong>
# \x00\x00\x00\x00\x1E Which LED to enable(LED1..4 << 1, 0x20=none)
# \xff\x27\x10\x00\x32 LED 1 (rate,duty factor
# \xff\x27\x10\x00\x32 LED 2
# \xff\x27\x10\x00\x32 LED 3
# \xff\x27\x10\x00\x32 LED 4
# \x00\x00\x00\x00\x00 Footer
# * the total time the led is active (0xff means forever)
# * | duty_length: how long a cycle is in deciseconds (0 means
"blink really fast")
# * | | ??? (Some form of phase shift or duty_length
multiplier?)
# * | | | % of duty_length the led is off (0xff means
100%)
# * | | | | % of duty_length the led is on (0xff
mean 100%)
# * | | | | |
# * 0xff, 0x27, 0x10, 0x00, 0x32,
--
Attached is a script which works with my Intec, but not with the SixAxis.
Simon
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rumble_pyusb.py --]
[-- Type: text/x-python; name="rumble_pyusb.py", Size: 2294 bytes --]
#!/usr/bin/python
#
# Small script to test the rumble on a PS3 gamepad
#
import usb
import time
rumble = 0
busses = usb.busses()
for bus in busses:
devices = bus.devices
for dev in devices:
if dev.idVendor == 0x054c and dev.idProduct == 0x0268:
print "Found RumblePad"
rumble = dev
if rumble == 0:
print "RumblePad not found"
exit(0)
# Get a device handler for the usb device and detach it from the kernel
dh = rumble.open()
dh.detachKernelDriver(0)
dh.claimInterface(0)
#command='\x52\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x1E\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\x00\x00\x00\x00\x00'
# \x01 Header
# \x00\x00\x00\x00\x00 ForceFeedback (\x00\xfe\x00+<rumble,40=weak,80=strong>
# \x00\x00\x00\x00\x1E Which LED to enable(LED1..4 << 1, 0x20=none)
# \xff\x27\x10\x00\x32 LED 1 (rate,duty factor
# \xff\x27\x10\x00\x32 LED 2
# \xff\x27\x10\x00\x32 LED 3
# \xff\x27\x10\x00\x32 LED 4
# \x00\x00\x00\x00\x00 Footer
# * the total time the led is active (0xff means forever)
# * | duty_length: how long a cycle is in deciseconds (0 means "blink really fast")
# * | | ??? (Some form of phase shift or duty_length multiplier?)
# * | | | % of duty_length the led is off (0xff means 100%)
# * | | | | % of duty_length the led is on (0xff mean 100%)
# * | | | | |
# * 0xff, 0x27, 0x10, 0x00, 0x32,
print "LED3 + LED0"
command='\x01' + \
'\x00\xfe\x00\xfe\x00' + \
'\x00\x00\x00\x00\x12' + \
'\xff\x00\x00\x00\x80' + \
'\xff\x00\x00\x00\x80' + \
'\xff\x00\x00\x00\x00' + \
'\xff\x00\x00\x00\x00' + \
'\x00\x00\x00\x00\x00'
dh.bulkWrite(0x02, command, len(command))
time.sleep(3)
print "LED2 + LED1"
command='\x01' + \
'\x00\xfe\x00\xfe\x00' + \
'\x00\x00\x00\x00\x0C' + \
'\xff\x27\x10\x80\x32' + \
'\xff\x27\x10\x80\x32' + \
'\xff\x27\x10\x00\x32' + \
'\xff\x27\x10\x00\x32' + \
'\x00\x00\x00\x00\x00'
dh.bulkWrite(0x02, command, len(command))
time.sleep(3)
print "no LEDs"
command='\x01' + \
'\x00\x00\x00\x00\x00' + \
'\x00\x00\x00\x00\x20' + \
'\xff\x07\x10\xff\x00' + \
'\xff\x07\x10\xff\x00' + \
'\xff\x07\x10\xff\x00' + \
'\xff\x07\x10\xff\x00' + \
'\x00\x00\x00\x00\x00'
dh.bulkWrite(0x02, command, len(command))
^ permalink raw reply
* Re: [PATCHv3 1/2] Input: twl4030-keypad - add device tree support
From: Dmitry Torokhov @ 2013-11-18 3:58 UTC (permalink / raw)
To: Pavel Machek
Cc: linux-input, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Grant Likely,
devicetree, linux-doc, linux-kernel
In-Reply-To: <20131117182840.GA30012@xo-6d-61-c0.localdomain>
On Sun, Nov 17, 2013 at 07:28:40PM +0100, Pavel Machek wrote:
> Hi!
>
> > On Mon, Nov 11, 2013 at 11:19:41PM +0100, Pavel Machek wrote:
> > > > + if (of_get_property(np, "linux,input-no-autorepeat", NULL))
> > > > + keypad_data->no_autorepeat = true;
> > >
> > > From 2/2:
> > >
> > > +Optional Properties specific to linux:
> > > +- linux,keypad-no-autorepeat: do not enable autorepeat feature.
> > >
> > > I'm confused now.
> >
> > good catch! That happens when one tries to mimic other drivers :/
> >
> > I just checked all DT input drivers for autorepeat keyword:
> >
> > DRIVER CODE DOCUMENTATION
> > twl4030-keypad linux,input-no-autorepeat linux,keypad-no-autorepeat
> > omap4-keypad linux,input-no-autorepeat linux,keypad-no-autorepeat
> > samsung-keypad linux,input-no-autorepeat linux,keypad-no-autorepeat
> > stmpe-keypad st,no-autorepeat st,no-autorepeat
> > spear-keyboard autorepeat autorepeat
> > tca8418-keypad keypad,autorepeat --- not documented ---
> > gpio-matrix-keypad linux,no-autorepeat linux,no-autorepeat
> > gpio-keys-polled autorepeat autorepeat
> > gpio-keys autorepeat --- no documentation ---
> >
> > Any suggestions how to continue fixing this mess? I guess first of
> > all the documentation of omap4-keypad, samsung-keypad and of course
> > the new twl4030-keypad driver should be fixed.
> >
> > Next it would be nice to choose one standard property name for this
> > and use it for twl4030-keypad. I suggest to use "linux,input-no-autorepeat".
>
> I'd suggest just simple "autorepeat", so that we get rid of ugly double-negation.
The idea was that majority of setups want autorepeat so in the absence
of the property autorepeat is turned on.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 00/31] ARM: tegra: use common reset and DMA bindings
From: Terje Bergström @ 2013-11-18 8:24 UTC (permalink / raw)
To: Stephen Warren
Cc: Mark Rutland, alsa-devel@alsa-project.org,
linux-usb@vger.kernel.org, Wolfram Sang, David Airlie,
linux-pci@vger.kernel.org, dri-devel@lists.freedesktop.org,
Marc Dietrich, linux-tegra@vger.kernel.org,
linux-i2c@vger.kernel.org, ac100@lists.launchpad.net,
devel@driverdev.osuosl.org, Stephen Warren, Alan Stern,
linux-serial@vger.kernel.org, linux-input@vger.kernel.org,
Thierry Reding, devicetree
In-Reply-To: <1384548866-13141-1-git-send-email-swarren@wwwdotorg.org>
On 15.11.2013 22:53, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> This series implements a common reset framework driver for Tegra, and
> updates all relevant Tegra drivers to use it. It also removes the custom
> DMA bindings and replaced them with the standard DMA DT bindings.
>
> Historically, the Tegra clock driver has exported a custom API for module
> reset. This series removes that API, and transitions DT and drivers to
> the new reset framework.
>
> The custom API used a "struct clk" to identify which module to reset, and
> consequently some DT bindings and drivers required clocks to be provided
> where they really needed just a reset identifier instead. Due to this
> known deficiency, I have always considered most Tegra bindings to be
> unstable. This series removes this excuse for instability, although I
> still consider some Tegra bindings unstable due to the need to convert to
> the common DMA bindings.
>
> Historically, Tegra DMA channels have been represented in DT using a
> custom nvidia,dma-request-selector property. Now that standard DMA DT
> bindings exist, convert all Tegra bindings, DTs, and drivers to use the
> standard instead.
>
> This series makes a DT-ABI-incompatible change to:
> - Require reset specifiers in DT where relevant.
> - Require standard DMA specifiers.
> - Remove clock specifiers from DT where they were only needed for reset.
> - Remove legacy DMA specifier properties.
>
> I anticipate merging this whole series into the Tegra and arm-soc trees
> as its own branch, due to internal dependencies. This branch will be
> stable and can then be merged into any other subsystem trees should any
> conflicts arise.
>
> This series depends on Peter's Tegra clock driver rework, available at
> git://nv-tegra.nvidia.com/user/pdeschrijver/linux tegra-clk-tegra124-0
> (or whatever version of that gets included in 3.14)
Overall, a good change. For host1x part:
Acked-By: Terje Bergstrom <tbergstrom@nvidia.com>
This patch does not change the behavior, but we have in original code
the problem that we don't flush the MC queue when resetting an engine.
This can cause some memory writes to not hit memory. There was an
earlier discussion on that, but we seem to have lost track of the issue.
Terje
^ permalink raw reply
* Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: Antonio Ospite @ 2013-11-18 10:27 UTC (permalink / raw)
To: simon; +Cc: Sven Eckelmann, linux-input, Jiri Kosina, Colin Leitner
In-Reply-To: <0d5dd16c7b4799f333fd4b4077e6c75c.squirrel@mungewell.org>
On Sun, 17 Nov 2013 20:21:04 -0500
simon@mungewell.org wrote:
[...]
> I note that the LED class directory is based on USB ID and node, which
> changes each time the controller is plugged in. This makes assigning LEDs
> in game configurations a pain.
>
> From my point of view a 'fixed UID' (such as a serial number) would be
> better, but most controllers don't report these. Any suggestions?
>
The bluez plugin uses libudev, so it knows how to address the right
device when setting the LED.
Ciao,
Antonio
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
^ permalink raw reply
* Re: [PATCH 1/2] input: touchscreen: fix spelling mistake in TSC/ADC DT binding
From: Mark Rutland @ 2013-11-18 11:40 UTC (permalink / raw)
To: Felipe Balbi
Cc: Sebastian Andrzej Siewior, dmitry.torokhov@gmail.com,
rob.herring@calxeda.com, Pawel Moll, swarren@wwwdotorg.org,
ijc+devicetree@hellion.org.uk, rob@landley.net,
bcousson@baylibre.com, Tony Lindgren, devicetree@vger.kernel.org,
Linux OMAP Mailing List, linux-input@vger.kernel.org
In-Reply-To: <20131115175356.GJ4698@saruman.home>
On Fri, Nov 15, 2013 at 05:53:56PM +0000, Felipe Balbi wrote:
> Hi,
>
> On Fri, Nov 15, 2013 at 03:55:40PM +0000, Mark Rutland wrote:
> > > > > > > diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
> > > > > > > index e1c5300..b61df9d 100644
> > > > > > > --- a/drivers/input/touchscreen/ti_am335x_tsc.c
> > > > > > > +++ b/drivers/input/touchscreen/ti_am335x_tsc.c
> > > > > > > @@ -348,9 +348,16 @@ static int titsc_parse_dt(struct platform_device *pdev,
> > > > > > > if (err < 0)
> > > > > > > return err;
> > > > > > >
> > > > > > > - err = of_property_read_u32(node, "ti,coordiante-readouts",
> > > > > > > + /*
> > > > > > > + * try with new binding first. If it fails, still try with
> > > > > > > + * bogus, miss-spelled version.
> > > > > > > + */
> > > > > > > + err = of_property_read_u32(node, "ti,coordinate-readouts",
> > > > > > > &ts_dev->coordinate_readouts);
> > > > > > > if (err < 0)
> > > > > > > + err = of_property_read_u32(node, "ti,coordiante-readouts",
> > > > > > > + &ts_dev->coordinate_readouts);
> > > > > > > + if (err < 0)
> > > > > > > return err;
> > > > > >
> > > > > > Thanks, very good. Do we keep this fallback for ever or just for a year
> > > > > > or two?
> > > > >
> > > > > That's for DT maintainers to decide but considering DT is an ABI, I
> > > > > guess we need to keep for 30 years or so :-p
> > > >
> > > > We keep it as long as we have to. If no-one's relying on the typo by the
> > > > next merge window, I see no reason we'd have to keep support for the
> > >
> > > and how could you know that ? considering it's an ABI, how could you
> > > ever know that ?
> >
> > If you know that the only user of a binding is a dts for a particular
> > product that you're in charge of, then you'd know the set of kernel +
> > dtb combinations out there, and can judge.
>
> once the binding has made into mainline, it's next to impossible to
> figure out who has downloaded a tarball containing that driver and made
> a product out of it.
>
> Besides keeping that check in the driver won't hurt at all in the long
> run. I would give it at least until 4.0 before thinking about removing,
> and that might still not be enough time.
That sounds sensible to me. As mentioned before I'd recommend adding a
warning for the typo now in the (possibly naïve) hope that it will
encourage people to fix up their dts early.
Thanks,
Mark.
--
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
^ permalink raw reply
* Re: [PATCH v3] add sur40 driver for Samsung SUR40 (aka MS Surface 2.0/Pixelsense)
From: Florian Echtler @ 2013-11-18 14:50 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Dmitry Torokhov, linux-input, benjamin.tissoires, dh.herrmann
In-Reply-To: <20131116180838.GA21591@polaris.bitmath.org>
[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]
Hello Henrik,
thanks for the info, sorry for me being such a nuisance :-)
I'll submit the patch to the Ubuntu kernel team once it's merged into
Dmitry's tree, would be great to see this in the next LTS version.
Best regards, Florian
On 16.11.2013 19:08, Henrik Rydberg wrote:
> Hi Florian,
>
>> Looks like the sur40 driver didn't make it into the 3.13 pull request,
>> anything still missing/broken? I was hoping this might still make it into
>> Ubuntu 14.04...
>
> Nothing is broken, don't worry. If you want to make sure to hit a
> certain merge window, then everything should be set and applied
> several weeks before that, to allow for automatic testing and
> administration. It could be that Dmitry is stacking up for a second
> pull request in a couple of weeks, but I would not count on it. If you
> want Ubuntu to pick your patches up, you can always try to send them
> to kernel-team@lists.ubuntu.com; they might be willing to carry your
> patches to the next kernel release. It helps to refer to the
> appropriate commits in linux-next.
>
> Thanks,
> Henrik
>
--
SENT FROM MY DEC VT50 TERMINAL
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: Antonio Ospite @ 2013-11-18 15:27 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: simon, linux-input, Jiri Kosina, Colin Leitner
In-Reply-To: <5434362.OvDlhOjSJd@sven-edge>
On Mon, 18 Nov 2013 00:12:14 +0100
Sven Eckelmann <sven@narfation.org> wrote:
> On Sunday 17 November 2013 23:25:59 Antonio Ospite wrote:
> > Sven, if you are going to resubmit another patch for this functionality
> > (I've seen your v2 just before hitting "Send"), wouldn't it be better
> > to advertise just FF_RUMBLE? AFAICS your first patch results in this
> > (from evtest):
> >
> > ....
> > Event type 21 (EV_FF)
> > Event code 80 (FF_RUMBLE)
> > Event code 81 (FF_PERIODIC)
> > Event code 88 (FF_SQUARE)
> > Event code 89 (FF_TRIANGLE)
> > Event code 90 (FF_SINE)
> > Event code 96 (FF_GAIN)
>
> I don't set them, ff_memless is doing that in input_ff_create_memless:
>
> /* we can emulate periodic effects with RUMBLE */
> if (test_bit(FF_RUMBLE, ff->ffbit)) {
> set_bit(FF_PERIODIC, dev->ffbit);
> set_bit(FF_SINE, dev->ffbit);
> set_bit(FF_TRIANGLE, dev->ffbit);
> set_bit(FF_SQUARE, dev->ffbit);
> }
>
I see now, thanks.
> > Also please make sure that setting the rumble does not change the LEDs
> > status if there is any set: HID output report 1 is used for both LEDs
> > and rumble. In the bluez plugin[1] I plan on setting LEDs from userspace
> > to match the joystick number, just as the PS3 does, it would be strange
> > for the user if a rumble event would reset the LEDs status.
>
> I never used the LEDs and therefore cannot say anything about it (I don't have
> a specification for the used command format). Maybe I can try to play with
> them next week.
>
> But you're patch has some comments in set_leds. Do I correctly interpret the
> byte 10 in leds_report as "only make changes to following LEDs"? So setting it
> to 1 would make the command not change the LEDs at all?
>
That would be an interesting approach, I'll do some experiment about
that too.
> > Maybe only send up to the rumble related bytes, or do a
> > read-modify-write if that is possible with HID output reports, if this
> > cannot be done we will have to design a solution to set LEDs too in the
> > kernel, in order to be able to keep some status around.
>
> The in-kernel state seems to be interesting because it is already done for the
> Sony Buzz LEDs. But I this is only a wild guess because I've never checked
> this code path and never used it.
>
> > Last comment, if we want a conditional config what about using
> > CONFIG_HID_SONY_FF instead of CONFIG_SONY_FF? Not a big deal of course,
> > just a suggestion.
>
> Most other hid devices omit the HID_ part in the _FF setting. This is also the
> reason why I've removed it too.
>
OK, I didn't know, I guess I am fine with that if others are doing so
too.
> $ grep 'config ' ./drivers/hid/Kconfig|grep -B1 _FF
[...]
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
^ permalink raw reply
* [PATCH] input: touchscreen: ti_am335x_tsc: warn about incorrect spelling
From: Felipe Balbi @ 2013-11-18 15:29 UTC (permalink / raw)
To: Mark Rutland
Cc: Sebastian Andrzej Siewior, dmitry.torokhov, rob.herring,
Pawel.Moll, swarren, ijc+devicetree, bcousson, Tony Lindgren,
devicetree, Linux OMAP Mailing List, linux-input, Felipe Balbi
In-Reply-To: <20131118114029.GD30853@e106331-lin.cambridge.arm.com>
In the hopes that people run new kernels on
their devices, let's add a warning message
asking users to have their DTS file fixed.
The goal is that by Linux 4.0 we will be
able to remove support for the bogus version
of our touchscreen's DTS.
Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
Here you go, I've added your Suggested-by Mark,
if you wish I can remove or change to something
else.
cheers
drivers/input/touchscreen/ti_am335x_tsc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index b61df9d..91302cd 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -354,9 +354,12 @@ static int titsc_parse_dt(struct platform_device *pdev,
*/
err = of_property_read_u32(node, "ti,coordinate-readouts",
&ts_dev->coordinate_readouts);
- if (err < 0)
+ if (err < 0) {
+ dev_warn(&pdev->dev, "please use 'ti,coordinate-readouts' instead\n");
err = of_property_read_u32(node, "ti,coordiante-readouts",
&ts_dev->coordinate_readouts);
+ }
+
if (err < 0)
return err;
--
1.8.4.GIT
^ permalink raw reply related
* Re: [PATCH] input: touchscreen: ti_am335x_tsc: warn about incorrect spelling
From: Mark Rutland @ 2013-11-18 15:35 UTC (permalink / raw)
To: Felipe Balbi
Cc: Sebastian Andrzej Siewior, dmitry.torokhov@gmail.com,
rob.herring@calxeda.com, Pawel Moll, swarren@wwwdotorg.org,
ijc+devicetree@hellion.org.uk, bcousson@baylibre.com,
Tony Lindgren, devicetree@vger.kernel.org,
Linux OMAP Mailing List, linux-input@vger.kernel.org
In-Reply-To: <1384788541-16338-1-git-send-email-balbi@ti.com>
On Mon, Nov 18, 2013 at 03:29:01PM +0000, Felipe Balbi wrote:
> In the hopes that people run new kernels on
> their devices, let's add a warning message
> asking users to have their DTS file fixed.
>
> The goal is that by Linux 4.0 we will be
> able to remove support for the bogus version
> of our touchscreen's DTS.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>
> Here you go, I've added your Suggested-by Mark,
> if you wish I can remove or change to something
> else.
>
> cheers
Looks fine to me, feel free to add my Ack.
Thanks,
Mark.
>
> drivers/input/touchscreen/ti_am335x_tsc.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
> index b61df9d..91302cd 100644
> --- a/drivers/input/touchscreen/ti_am335x_tsc.c
> +++ b/drivers/input/touchscreen/ti_am335x_tsc.c
> @@ -354,9 +354,12 @@ static int titsc_parse_dt(struct platform_device *pdev,
> */
> err = of_property_read_u32(node, "ti,coordinate-readouts",
> &ts_dev->coordinate_readouts);
> - if (err < 0)
> + if (err < 0) {
> + dev_warn(&pdev->dev, "please use 'ti,coordinate-readouts' instead\n");
> err = of_property_read_u32(node, "ti,coordiante-readouts",
> &ts_dev->coordinate_readouts);
> + }
> +
> if (err < 0)
> return err;
>
> --
> 1.8.4.GIT
>
>
^ permalink raw reply
* Re: About the PS3 Sixaxis linux driver
From: Benjamin Tissoires @ 2013-11-18 15:38 UTC (permalink / raw)
To: Antonio Ospite, simon; +Cc: Henrik Rydberg, case, linux-input, Dmitry Torokhov
In-Reply-To: <20131116180607.8508c13158a181f8c5fa7dd6@studenti.unina.it>
Hi guys,
I forgot to say that:
- IMO, the approach raised in 2011 by Antonio[1] was the correct one:
the multitouch protocol introduced a regression with devices providing
large axis number, so we should fix this in input core, not on a per
device basis.
- I'll send an update to this patch later this week.
- I don't think remapping the PS3 axis now will make into upstream
(backward compatibility), but this is only my own opinion
- I have discussed this with the Xinput maintainer, and he also would
like to get the information from the kernel when the ABS_MT axis are
used as ABS_MT or ABS_MISC, so I'll also add a property in this way.
Cheers,
Benjamin
[1] http://www.spinics.net/lists/linux-input/msg16881.html
On 16/11/13 12:06, Antonio Ospite wrote:
> On Mon, 4 Nov 2013 16:40:29 -0500
> simon@mungewell.org wrote:
>
> Hi Simon, sorry for the delay on this one.
>
>>> Actually I intended to rewrite the whole HID descriptor and add a sane
>>> mapping for all the buttons (maybe following the Gamepad API and
>>> even using the new ABS_ACCEL_* and ABS_GYRO_* events from David
>>> Hermann).
>>
>> I think that the root of the problem here is simply the large number of
>> axis, and that the keycodes start at 'axis-x' then overflow into/past the
>> MultiTouch keycode.
>>
>
> Yes, and the code that maps HID usages to event keycodes also comes into
> play I guess.
>
>> This problem is going to be a more common one as HID drivers present more
>> buttons/axis.
>>
>> We did some patching here:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-sony.c?id=refs/tags/v3.12#n301
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/hid/hid-sony.c?id=61ab44bebdefab296487e7cd723a634849278827
>>
>> Do we need to fix this is the HID descriptor, or in 'hid-sony' input stuff?
>
> I think both:
>
> 1. adjust the HID descriptor into something that can be more easily
> remapped;
>
> 2. remap the keycodes in hid-sony to those of the gamepad API, because
> AFAICT that can't be achieved by solely deciding HID usage codes in
> the descriptor.
>
>> Do you have example of another driver to copy?
>
> I am doing some work already in this direction, on the lines of what we
> did for the ps3remote in hid-sony, but it needs some input bits not
> merged yet, I'll send an RFC as soon as this becomes more easily
> testable.
>
> Regards,
> Antonio
>
^ permalink raw reply
* Re: About the PS3 Sixaxis linux driver
From: Antonio Ospite @ 2013-11-18 16:25 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: simon, Henrik Rydberg, case, linux-input, Dmitry Torokhov,
David Herrmann
In-Reply-To: <528A3482.8040807@redhat.com>
On Mon, 18 Nov 2013 10:38:42 -0500
Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
> Hi guys,
>
> I forgot to say that:
> - IMO, the approach raised in 2011 by Antonio[1] was the correct one:
> the multitouch protocol introduced a regression with devices providing
> large axis number, so we should fix this in input core, not on a per
> device basis.
> - I'll send an update to this patch later this week.
Thanks.
> - I don't think remapping the PS3 axis now will make into upstream
> (backward compatibility), but this is only my own opinion
What do other people think? I was going to motivate the change of
keycodes with the fact that the device was not fully functional for a
long time, but if you fix it in input core this argument does not hold
anymore :)
It would have been a good excuse to have a user of the full gamepad
API and of a good part of the motion-tracking API, but I won't push
this too hard.
> - I have discussed this with the Xinput maintainer, and he also would
> like to get the information from the kernel when the ABS_MT axis are
> used as ABS_MT or ABS_MISC, so I'll also add a property in this way.
>
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
^ permalink raw reply
* Re: About the PS3 Sixaxis linux driver
From: Benjamin Tissoires @ 2013-11-18 17:02 UTC (permalink / raw)
To: Antonio Ospite
Cc: simon, Henrik Rydberg, case, linux-input, Dmitry Torokhov,
David Herrmann
In-Reply-To: <20131118172535.e6abd12d338023ead6477058@studenti.unina.it>
On 18/11/13 11:25, Antonio Ospite wrote:
> On Mon, 18 Nov 2013 10:38:42 -0500
> Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
>
>> Hi guys,
>>
>> I forgot to say that:
>> - IMO, the approach raised in 2011 by Antonio[1] was the correct one:
>> the multitouch protocol introduced a regression with devices providing
>> large axis number, so we should fix this in input core, not on a per
>> device basis.
>> - I'll send an update to this patch later this week.
>
> Thanks.
>
>> - I don't think remapping the PS3 axis now will make into upstream
>> (backward compatibility), but this is only my own opinion
>
> What do other people think? I was going to motivate the change of
> keycodes with the fact that the device was not fully functional for a
> long time, but if you fix it in input core this argument does not hold
> anymore :)
Just FYI, I mentioned this because there already has been 2 or 3 threads
regarding bad uses of keycodes/axis for gamepads (WII, XBox, and PS3
IIRC), and the result was always "we do not break user space".
Personally, I have nothing against mapping the correct axis :)
>
> It would have been a good excuse to have a user of the full gamepad
> API and of a good part of the motion-tracking API, but I won't push
> this too hard.
There are some initiatives in the user space to fix those kernel
problems. libinputmapper (find one of David Herrmann's post) and
libevdev (on Freedesktop) are some good candidates.
Cheers,
Benjamin
^ permalink raw reply
* Re: Synaptics i2c-hid touchpad driver
From: Benjamin Tissoires @ 2013-11-18 19:32 UTC (permalink / raw)
To: Andrew Duggan; +Cc: Wouter van der Graaf, Jiri Kosina, linux-input
In-Reply-To: <528A6991.2010604@synaptics.com>
Alright, here we go... \o/
following the thread
http://www.spinics.net/lists/linux-input/msg27737.html
For the record, the Dell XPS12 from 2013 has a current driver under
development for its touchpad.
It is located here:
https://github.com/mightybigcar/synaptics-rmi4/commits/rmihid
As mentioned by Andrew, any feedback are welcome.
Cheers,
Benjamin
On Mon, Nov 18, 2013 at 2:25 PM, Andrew Duggan <aduggan@synaptics.com> wrote:
> Hi Benjamin and Wouter,
>
> Adding linux-input to this thread is fine with me. The plan is to submit the
> core of the rmi4 driver and then submitting the HID pieces. But, having
> people who are interested in the driver look it over in the mean time would
> be good. Especially, now that there is hardware on the market.
>
> Andrew
>
>
> On 11/18/2013 07:48 AM, Benjamin Tissoires wrote:
>>
>> Hi Wouter,
>>
>> sorry, I should have kept you informed.
>>
>> I have asked Andrew from Synaptics to know what was their plan for
>> this. He told me that they are still working on it.
>>
>> He also gave me the link to the development tree of this work:
>> https://github.com/mightybigcar/synaptics-rmi4/commits/rmihid
>>
>> I have tested it, and it works even if there is some tiny adjustments
>> to do before taking this upstream.
>>
>> Andrew, I was not sure if we could add the linux-input LKML to this
>> thread (don't know how much private this repo is). If so, I think some
>> other users would be interested by this information, and we should
>> open it.
>>
>> Cheers,
>> Benjamin
>>
>> On Mon, Nov 18, 2013 at 5:37 AM, Wouter van der Graaf <wouter@dynora.nl>
>> wrote:
>>>
>>> Hi Benjamin,
>>>
>>> Just curious; is there any progress on the Synaptics i2c-hid touchpad
>>> driver? Is it still on the radar? Can I contribute in some way?
>>>
>>> Thank you!
>>>
>>> Wouter
>>>
>>> --
>>> Wouter van der Graaf
>>> Open technologist / Partner
>>>
>>> d y n o r a
>>> Innovative software development
>>>
>>> +31 6 2893 0615 | wouter@dynora.nl
>>>
>>> Burg. de Bruïnelaan 95 | 3331 AC Zwijndrecht | NL
>>> +31 78 843 3139 | contact@dynora.nl | www.dynora.nl
>
>
--
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
^ permalink raw reply
* [PATCHv3 0/5] HID: sony: Dualshock3 USB FF deadlock fix and LED support
From: Sven Eckelmann @ 2013-11-18 20:42 UTC (permalink / raw)
To: linux-input; +Cc: Jiri Kosina, Colin Leitner
Hi,
this replaces the previously sent patches:
* [PATCH] Revert "HID: sony: Add force feedback support for Dualshock3 USB"
* [PATCHv2] HID: sony: Send FF commands in non-atomic context
I've resent the first patch again as v3 because some function/variable names
were changed to better integrate in the LED support patches. It is sent as part
of this patchset because these changes require the sony_state_worker to change
the state of the LEDs.
Kind regards,
Sven
^ permalink raw reply
* [PATCHv3 1/5] HID: sony: Send ff commands in non-atomic context
From: Sven Eckelmann @ 2013-11-18 20:42 UTC (permalink / raw)
To: linux-input; +Cc: Jiri Kosina, Colin Leitner, Sven Eckelmann
In-Reply-To: <1384807356-15561-1-git-send-email-sven@narfation.org>
The ff_memless has a timer running which gets run in an atomic context and
calls the play_effect callback. The callback function for sony uses the
hid_output_raw_report (overwritten by sixaxis_usb_output_raw_report) function
to handle differences in the control message format. It is not safe for an
atomic context because it may sleep later in usb_start_wait_urb.
This "scheduling while atomic" can cause the system to lock up. A workaround is
to make the force feedback state update using work_queues and use the
play_effect function only to enqueue the work item.
This problem was introduced in a08c22c0df0ad23d0df10ae1a9df26643589b3cc
("HID: sony: Add force feedback support for Dualshock3 USB").
Reported-by: Simon Wood <simon@mungewell.org>
Reported-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
drivers/hid/hid-sony.c | 53 +++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 42 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index da551d1..28b847a 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -225,6 +225,13 @@ static const unsigned int buzz_keymap[] = {
struct sony_sc {
unsigned long quirks;
+#ifdef CONFIG_SONY_FF
+ struct work_struct state_worker;
+ struct hid_device *hdev;
+ __u8 left;
+ __u8 right;
+#endif
+
void *extra;
};
@@ -615,9 +622,9 @@ static void buzz_remove(struct hid_device *hdev)
}
#ifdef CONFIG_SONY_FF
-static int sony_play_effect(struct input_dev *dev, void *data,
- struct ff_effect *effect)
+static void sony_state_worker(struct work_struct *work)
{
+ struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
unsigned char buf[] = {
0x01,
0x00, 0xff, 0x00, 0xff, 0x00,
@@ -628,21 +635,28 @@ static int sony_play_effect(struct input_dev *dev, void *data,
0xff, 0x27, 0x10, 0x00, 0x32,
0x00, 0x00, 0x00, 0x00, 0x00
};
- __u8 left;
- __u8 right;
+
+ buf[3] = sc->right;
+ buf[5] = sc->left;
+
+ sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf),
+ HID_OUTPUT_REPORT);
+}
+
+static int sony_play_effect(struct input_dev *dev, void *data,
+ struct ff_effect *effect)
+{
struct hid_device *hid = input_get_drvdata(dev);
+ struct sony_sc *sc = hid_get_drvdata(hid);
if (effect->type != FF_RUMBLE)
return 0;
- left = effect->u.rumble.strong_magnitude / 256;
- right = effect->u.rumble.weak_magnitude ? 1 : 0;
+ sc->left = effect->u.rumble.strong_magnitude / 256;
+ sc->right = effect->u.rumble.weak_magnitude ? 1 : 0;
- buf[3] = right;
- buf[5] = left;
-
- return hid->hid_output_raw_report(hid, buf, sizeof(buf),
- HID_OUTPUT_REPORT);
+ schedule_work(&sc->state_worker);
+ return 0;
}
static int sony_init_ff(struct hid_device *hdev)
@@ -650,16 +664,31 @@ static int sony_init_ff(struct hid_device *hdev)
struct hid_input *hidinput = list_entry(hdev->inputs.next,
struct hid_input, list);
struct input_dev *input_dev = hidinput->input;
+ struct sony_sc *sc = hid_get_drvdata(hdev);
+
+ sc->hdev = hdev;
+ INIT_WORK(&sc->state_worker, sony_state_worker);
input_set_capability(input_dev, EV_FF, FF_RUMBLE);
return input_ff_create_memless(input_dev, NULL, sony_play_effect);
}
+static void sony_destroy_ff(struct hid_device *hdev)
+{
+ struct sony_sc *sc = hid_get_drvdata(hdev);
+
+ cancel_work_sync(&sc->state_worker);
+}
+
#else
static int sony_init_ff(struct hid_device *hdev)
{
return 0;
}
+
+static void sony_destroy_ff(struct hid_device *hdev)
+{
+}
#endif
static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
@@ -728,6 +757,8 @@ static void sony_remove(struct hid_device *hdev)
if (sc->quirks & BUZZ_CONTROLLER)
buzz_remove(hdev);
+ sony_destroy_ff(hdev);
+
hid_hw_stop(hdev);
}
--
1.8.4.3
^ permalink raw reply related
* [PATCHv3 2/5] HID: sony: Use BIT(x) macro to define quirks constants
From: Sven Eckelmann @ 2013-11-18 20:42 UTC (permalink / raw)
To: linux-input; +Cc: Jiri Kosina, Colin Leitner, Sven Eckelmann
In-Reply-To: <1384807356-15561-1-git-send-email-sven@narfation.org>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
drivers/hid/hid-sony.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 28b847a..0677292 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -33,11 +33,11 @@
#include "hid-ids.h"
-#define VAIO_RDESC_CONSTANT (1 << 0)
-#define SIXAXIS_CONTROLLER_USB (1 << 1)
-#define SIXAXIS_CONTROLLER_BT (1 << 2)
-#define BUZZ_CONTROLLER (1 << 3)
-#define PS3REMOTE (1 << 4)
+#define VAIO_RDESC_CONSTANT BIT(0)
+#define SIXAXIS_CONTROLLER_USB BIT(1)
+#define SIXAXIS_CONTROLLER_BT BIT(2)
+#define BUZZ_CONTROLLER BIT(3)
+#define PS3REMOTE BIT(4)
static const u8 sixaxis_rdesc_fixup[] = {
0x95, 0x13, 0x09, 0x01, 0x81, 0x02, 0x95, 0x0C,
--
1.8.4.3
^ permalink raw reply related
* [PATCHv3 3/5] HID: sony: Rename buzz_* functions to sony_led_*
From: Sven Eckelmann @ 2013-11-18 20:42 UTC (permalink / raw)
To: linux-input; +Cc: Jiri Kosina, Colin Leitner, Sven Eckelmann
In-Reply-To: <1384807356-15561-1-git-send-email-sven@narfation.org>
More controllers managed by the hid-sony module have 4 LEDs. These can share
most of the functionality provided by the buzz functions.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
drivers/hid/hid-sony.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 0677292..cdb2419 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -466,7 +466,7 @@ static void buzz_set_leds(struct hid_device *hdev, int leds)
hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
}
-static void buzz_led_set_brightness(struct led_classdev *led,
+static void sony_led_set_brightness(struct led_classdev *led,
enum led_brightness value)
{
struct device *dev = led->dev->parent;
@@ -498,7 +498,7 @@ static void buzz_led_set_brightness(struct led_classdev *led,
}
}
-static enum led_brightness buzz_led_get_brightness(struct led_classdev *led)
+static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
{
struct device *dev = led->dev->parent;
struct hid_device *hdev = container_of(dev, struct hid_device, dev);
@@ -525,7 +525,7 @@ static enum led_brightness buzz_led_get_brightness(struct led_classdev *led)
return on ? LED_FULL : LED_OFF;
}
-static int buzz_init(struct hid_device *hdev)
+static int sony_leds_init(struct hid_device *hdev)
{
struct sony_sc *drv_data;
struct buzz_extra *buzz;
@@ -567,8 +567,8 @@ static int buzz_init(struct hid_device *hdev)
led->name = name;
led->brightness = 0;
led->max_brightness = 1;
- led->brightness_get = buzz_led_get_brightness;
- led->brightness_set = buzz_led_set_brightness;
+ led->brightness_get = sony_led_get_brightness;
+ led->brightness_set = sony_led_set_brightness;
if (led_classdev_register(&hdev->dev, led)) {
hid_err(hdev, "Failed to register LED %d\n", n);
@@ -596,7 +596,7 @@ error_leds:
return ret;
}
-static void buzz_remove(struct hid_device *hdev)
+static void sony_leds_remove(struct hid_device *hdev)
{
struct sony_sc *drv_data;
struct buzz_extra *buzz;
@@ -733,7 +733,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
ret = sixaxis_set_operational_bt(hdev);
else if (sc->quirks & BUZZ_CONTROLLER)
- ret = buzz_init(hdev);
+ ret = sony_leds_init(hdev);
else
ret = 0;
@@ -755,7 +755,7 @@ static void sony_remove(struct hid_device *hdev)
struct sony_sc *sc = hid_get_drvdata(hdev);
if (sc->quirks & BUZZ_CONTROLLER)
- buzz_remove(hdev);
+ sony_leds_remove(hdev);
sony_destroy_ff(hdev);
--
1.8.4.3
^ permalink raw reply related
* [PATCHv3 4/5] HID: sony: Move LED data to the main structure
From: Sven Eckelmann @ 2013-11-18 20:42 UTC (permalink / raw)
To: linux-input; +Cc: Jiri Kosina, Colin Leitner, Sven Eckelmann
In-Reply-To: <1384807356-15561-1-git-send-email-sven@narfation.org>
It is not necessary to keep the LED information in an extra struct which is
only used by the Buzz device. It can also be used by other devices.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
drivers/hid/hid-sony.c | 58 +++++++++++++++-----------------------------------
1 file changed, 17 insertions(+), 41 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index cdb2419..2f93aab 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -223,6 +223,7 @@ static const unsigned int buzz_keymap[] = {
};
struct sony_sc {
+ struct led_classdev *leds[4];
unsigned long quirks;
#ifdef CONFIG_SONY_FF
@@ -232,12 +233,7 @@ struct sony_sc {
__u8 right;
#endif
- void *extra;
-};
-
-struct buzz_extra {
- int led_state;
- struct led_classdev *leds[4];
+ __u8 led_state;
};
static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
@@ -472,26 +468,24 @@ static void sony_led_set_brightness(struct led_classdev *led,
struct device *dev = led->dev->parent;
struct hid_device *hdev = container_of(dev, struct hid_device, dev);
struct sony_sc *drv_data;
- struct buzz_extra *buzz;
int n;
drv_data = hid_get_drvdata(hdev);
- if (!drv_data || !drv_data->extra) {
+ if (!drv_data) {
hid_err(hdev, "No device data\n");
return;
}
- buzz = drv_data->extra;
for (n = 0; n < 4; n++) {
- if (led == buzz->leds[n]) {
- int on = !! (buzz->led_state & (1 << n));
+ if (led == drv_data->leds[n]) {
+ int on = !!(drv_data->led_state & (1 << n));
if (value == LED_OFF && on) {
- buzz->led_state &= ~(1 << n);
- buzz_set_leds(hdev, buzz->led_state);
+ drv_data->led_state &= ~(1 << n);
+ buzz_set_leds(hdev, drv_data->led_state);
} else if (value != LED_OFF && !on) {
- buzz->led_state |= (1 << n);
- buzz_set_leds(hdev, buzz->led_state);
+ drv_data->led_state |= (1 << n);
+ buzz_set_leds(hdev, drv_data->led_state);
}
break;
}
@@ -503,21 +497,19 @@ static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
struct device *dev = led->dev->parent;
struct hid_device *hdev = container_of(dev, struct hid_device, dev);
struct sony_sc *drv_data;
- struct buzz_extra *buzz;
int n;
int on = 0;
drv_data = hid_get_drvdata(hdev);
- if (!drv_data || !drv_data->extra) {
+ if (!drv_data) {
hid_err(hdev, "No device data\n");
return LED_OFF;
}
- buzz = drv_data->extra;
for (n = 0; n < 4; n++) {
- if (led == buzz->leds[n]) {
- on = !! (buzz->led_state & (1 << n));
+ if (led == drv_data->leds[n]) {
+ on = !!(drv_data->led_state & (1 << n));
break;
}
}
@@ -528,7 +520,6 @@ static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
static int sony_leds_init(struct hid_device *hdev)
{
struct sony_sc *drv_data;
- struct buzz_extra *buzz;
int n, ret = 0;
struct led_classdev *led;
size_t name_sz;
@@ -541,13 +532,6 @@ static int sony_leds_init(struct hid_device *hdev)
if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
return -ENODEV;
- buzz = kzalloc(sizeof(*buzz), GFP_KERNEL);
- if (!buzz) {
- hid_err(hdev, "Insufficient memory, cannot allocate driver data\n");
- return -ENOMEM;
- }
- drv_data->extra = buzz;
-
/* Clear LEDs as we have no way of reading their initial state. This is
* only relevant if the driver is loaded after somebody actively set the
* LEDs to on */
@@ -576,49 +560,41 @@ static int sony_leds_init(struct hid_device *hdev)
goto error_leds;
}
- buzz->leds[n] = led;
+ drv_data->leds[n] = led;
}
return ret;
error_leds:
for (n = 0; n < 4; n++) {
- led = buzz->leds[n];
- buzz->leds[n] = NULL;
+ led = drv_data->leds[n];
+ drv_data->leds[n] = NULL;
if (!led)
continue;
led_classdev_unregister(led);
kfree(led);
}
- kfree(drv_data->extra);
- drv_data->extra = NULL;
return ret;
}
static void sony_leds_remove(struct hid_device *hdev)
{
struct sony_sc *drv_data;
- struct buzz_extra *buzz;
struct led_classdev *led;
int n;
drv_data = hid_get_drvdata(hdev);
BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
- buzz = drv_data->extra;
-
for (n = 0; n < 4; n++) {
- led = buzz->leds[n];
- buzz->leds[n] = NULL;
+ led = drv_data->leds[n];
+ drv_data->leds[n] = NULL;
if (!led)
continue;
led_classdev_unregister(led);
kfree(led);
}
-
- kfree(drv_data->extra);
- drv_data->extra = NULL;
}
#ifdef CONFIG_SONY_FF
--
1.8.4.3
^ permalink raw reply related
* [PATCHv3 5/5] HID: sony: Add LED support for Sixaxis/Dualshock3 USB
From: Sven Eckelmann @ 2013-11-18 20:42 UTC (permalink / raw)
To: linux-input; +Cc: Jiri Kosina, Colin Leitner, Sven Eckelmann
In-Reply-To: <1384807356-15561-1-git-send-email-sven@narfation.org>
The PS3 Sixaxis controller has 4 LEDs which can be controlled using the same
command as the rumble functionality. It seems not to be possible to only change
the LED without modifying the rumble motor state. Thus both have to be stored
on the host and retransmitted when either the LED or rumble state is changed.
Third party controllers may not support to disable all LEDs at once. These
controllers automatically switch to blinking of all LEDs when no LED is active
anymore.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Tested-by: Simon Wood <simon@mungewell.org>
---
drivers/hid/hid-sony.c | 118 +++++++++++++++++++++++++++++--------------------
1 file changed, 71 insertions(+), 47 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 2f93aab..b60bc38 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -39,6 +39,8 @@
#define BUZZ_CONTROLLER BIT(3)
#define PS3REMOTE BIT(4)
+#define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER)
+
static const u8 sixaxis_rdesc_fixup[] = {
0x95, 0x13, 0x09, 0x01, 0x81, 0x02, 0x95, 0x0C,
0x81, 0x01, 0x75, 0x10, 0x95, 0x04, 0x26, 0xFF,
@@ -223,12 +225,12 @@ static const unsigned int buzz_keymap[] = {
};
struct sony_sc {
+ struct hid_device *hdev;
struct led_classdev *leds[4];
unsigned long quirks;
+ struct work_struct state_worker;
#ifdef CONFIG_SONY_FF
- struct work_struct state_worker;
- struct hid_device *hdev;
__u8 left;
__u8 right;
#endif
@@ -462,6 +464,18 @@ static void buzz_set_leds(struct hid_device *hdev, int leds)
hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
}
+static void sony_set_leds(struct hid_device *hdev, __u8 leds)
+{
+ struct sony_sc *drv_data = hid_get_drvdata(hdev);
+
+ if (drv_data->quirks & BUZZ_CONTROLLER) {
+ buzz_set_leds(hdev, leds);
+ } else if (drv_data->quirks & SIXAXIS_CONTROLLER_USB) {
+ drv_data->led_state = leds;
+ schedule_work(&drv_data->state_worker);
+ }
+}
+
static void sony_led_set_brightness(struct led_classdev *led,
enum led_brightness value)
{
@@ -482,10 +496,10 @@ static void sony_led_set_brightness(struct led_classdev *led,
int on = !!(drv_data->led_state & (1 << n));
if (value == LED_OFF && on) {
drv_data->led_state &= ~(1 << n);
- buzz_set_leds(hdev, drv_data->led_state);
+ sony_set_leds(hdev, drv_data->led_state);
} else if (value != LED_OFF && !on) {
drv_data->led_state |= (1 << n);
- buzz_set_leds(hdev, drv_data->led_state);
+ sony_set_leds(hdev, drv_data->led_state);
}
break;
}
@@ -517,6 +531,25 @@ static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
return on ? LED_FULL : LED_OFF;
}
+static void sony_leds_remove(struct hid_device *hdev)
+{
+ struct sony_sc *drv_data;
+ struct led_classdev *led;
+ int n;
+
+ drv_data = hid_get_drvdata(hdev);
+ BUG_ON(!(drv_data->quirks & SONY_LED_SUPPORT));
+
+ for (n = 0; n < 4; n++) {
+ led = drv_data->leds[n];
+ drv_data->leds[n] = NULL;
+ if (!led)
+ continue;
+ led_classdev_unregister(led);
+ kfree(led);
+ }
+}
+
static int sony_leds_init(struct hid_device *hdev)
{
struct sony_sc *drv_data;
@@ -524,20 +557,29 @@ static int sony_leds_init(struct hid_device *hdev)
struct led_classdev *led;
size_t name_sz;
char *name;
+ size_t name_len;
+ const char *name_fmt;
drv_data = hid_get_drvdata(hdev);
- BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
+ BUG_ON(!(drv_data->quirks & SONY_LED_SUPPORT));
- /* Validate expected report characteristics. */
- if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
- return -ENODEV;
+ if (drv_data->quirks & BUZZ_CONTROLLER) {
+ name_len = strlen("::buzz#");
+ name_fmt = "%s::buzz%d";
+ /* Validate expected report characteristics. */
+ if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
+ return -ENODEV;
+ } else {
+ name_len = strlen("::sony#");
+ name_fmt = "%s::sony%d";
+ }
/* Clear LEDs as we have no way of reading their initial state. This is
* only relevant if the driver is loaded after somebody actively set the
* LEDs to on */
- buzz_set_leds(hdev, 0x00);
+ sony_set_leds(hdev, 0x00);
- name_sz = strlen(dev_name(&hdev->dev)) + strlen("::buzz#") + 1;
+ name_sz = strlen(dev_name(&hdev->dev)) + name_len + 1;
for (n = 0; n < 4; n++) {
led = kzalloc(sizeof(struct led_classdev) + name_sz, GFP_KERNEL);
@@ -547,7 +589,7 @@ static int sony_leds_init(struct hid_device *hdev)
}
name = (void *)(&led[1]);
- snprintf(name, name_sz, "%s::buzz%d", dev_name(&hdev->dev), n + 1);
+ snprintf(name, name_sz, name_fmt, dev_name(&hdev->dev), n + 1);
led->name = name;
led->brightness = 0;
led->max_brightness = 1;
@@ -566,45 +608,18 @@ static int sony_leds_init(struct hid_device *hdev)
return ret;
error_leds:
- for (n = 0; n < 4; n++) {
- led = drv_data->leds[n];
- drv_data->leds[n] = NULL;
- if (!led)
- continue;
- led_classdev_unregister(led);
- kfree(led);
- }
+ sony_leds_remove(hdev);
return ret;
}
-static void sony_leds_remove(struct hid_device *hdev)
-{
- struct sony_sc *drv_data;
- struct led_classdev *led;
- int n;
-
- drv_data = hid_get_drvdata(hdev);
- BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
-
- for (n = 0; n < 4; n++) {
- led = drv_data->leds[n];
- drv_data->leds[n] = NULL;
- if (!led)
- continue;
- led_classdev_unregister(led);
- kfree(led);
- }
-}
-
-#ifdef CONFIG_SONY_FF
static void sony_state_worker(struct work_struct *work)
{
struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
unsigned char buf[] = {
0x01,
0x00, 0xff, 0x00, 0xff, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x03,
+ 0x00, 0x00, 0x00, 0x00, 0x00,
0xff, 0x27, 0x10, 0x00, 0x32,
0xff, 0x27, 0x10, 0x00, 0x32,
0xff, 0x27, 0x10, 0x00, 0x32,
@@ -612,13 +627,18 @@ static void sony_state_worker(struct work_struct *work)
0x00, 0x00, 0x00, 0x00, 0x00
};
+#ifdef CONFIG_SONY_FF
buf[3] = sc->right;
buf[5] = sc->left;
+#endif
+
+ buf[10] |= (sc->led_state & 0xf) << 1;
sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf),
HID_OUTPUT_REPORT);
}
+#ifdef CONFIG_SONY_FF
static int sony_play_effect(struct input_dev *dev, void *data,
struct ff_effect *effect)
{
@@ -640,10 +660,6 @@ static int sony_init_ff(struct hid_device *hdev)
struct hid_input *hidinput = list_entry(hdev->inputs.next,
struct hid_input, list);
struct input_dev *input_dev = hidinput->input;
- struct sony_sc *sc = hid_get_drvdata(hdev);
-
- sc->hdev = hdev;
- INIT_WORK(&sc->state_worker, sony_state_worker);
input_set_capability(input_dev, EV_FF, FF_RUMBLE);
return input_ff_create_memless(input_dev, NULL, sony_play_effect);
@@ -682,6 +698,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
sc->quirks = quirks;
hid_set_drvdata(hdev, sc);
+ sc->hdev = hdev;
ret = hid_parse(hdev);
if (ret) {
@@ -705,23 +722,30 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
hdev->hid_output_raw_report = sixaxis_usb_output_raw_report;
ret = sixaxis_set_operational_usb(hdev);
+ INIT_WORK(&sc->state_worker, sony_state_worker);
}
else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
ret = sixaxis_set_operational_bt(hdev);
- else if (sc->quirks & BUZZ_CONTROLLER)
- ret = sony_leds_init(hdev);
else
ret = 0;
if (ret < 0)
goto err_stop;
+ if (sc->quirks & SONY_LED_SUPPORT) {
+ ret = sony_leds_init(hdev);
+ if (ret < 0)
+ goto err_stop;
+ }
+
ret = sony_init_ff(hdev);
if (ret < 0)
goto err_stop;
return 0;
err_stop:
+ if (sc->quirks & SONY_LED_SUPPORT)
+ sony_leds_remove(hdev);
hid_hw_stop(hdev);
return ret;
}
@@ -730,7 +754,7 @@ static void sony_remove(struct hid_device *hdev)
{
struct sony_sc *sc = hid_get_drvdata(hdev);
- if (sc->quirks & BUZZ_CONTROLLER)
+ if (sc->quirks & SONY_LED_SUPPORT)
sony_leds_remove(hdev);
sony_destroy_ff(hdev);
--
1.8.4.3
^ permalink raw reply related
* Re: [PATCHv2] HID: sony: Send FF commands in non-atomic context
From: Jiri Kosina @ 2013-11-19 8:38 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: linux-input, simon
In-Reply-To: <1384717101-7027-1-git-send-email-sven@narfation.org>
On Sun, 17 Nov 2013, Sven Eckelmann wrote:
> The ff_memless has a timer running which gets run in an atomic context and
> calls the play_effect callback. The callback function for sony uses the
> hid_output_raw_report (overwritten by sixaxis_usb_output_raw_report) function
> to handle differences in the control message format. It is not safe for an
> atomic context because it may sleep later in usb_start_wait_urb.
>
> This "scheduling while atomic" can cause the system to lock up. A workaround is
> to make the force feedback state update using work_queues and use the
> play_effect function only to enqueue the work item.
>
> Reported-by: Simon Wood <simon@mungewell.org>
> Reported-by: David Herrmann <dh.herrmann@gmail.com>
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> This patch can replace
> 'Revert "HID: sony: Add force feedback support for Dualshock3 USB"'
>
> It doesn't contain the command changes from <2014555.nmU692BQMt@sven-desktop>.
> It would be nice when Simon Wood could test it again with his Intec Wired
> controller. When it doesn't work:
>
> * Try to change to usb_interrupt_msg instead of sc->hdev->hid_output_raw_report
> * Send a interrupt message using buf[10] = 0x02
> (only when buf[3] != 0 || buf[5] != 0) followed by a message with
> buf[10] = 0x1e (always)
Now applied for 3.13. Thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCHv2] HID: sony: Send FF commands in non-atomic context
From: Sven Eckelmann @ 2013-11-19 10:02 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, simon
In-Reply-To: <alpine.LNX.2.00.1311190938020.6919@pobox.suse.cz>
[-- Attachment #1: Type: text/plain, Size: 302 bytes --]
On Tuesday 19 November 2013 09:38:21 Jiri Kosina wrote:
[...]
> Now applied for 3.13. Thanks.
Ok, but would have been nice when the v3 version have been applied. Now I have
to resubmit the LED patchset to include the function/variable rename. I will
do this tonight or tomorrow.
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: conflict of wiimote and hid-sony
From: Jiri Kosina @ 2013-11-19 10:30 UTC (permalink / raw)
To: David Herrmann; +Cc: Michel Kraus, open list:HID CORE LAYER
In-Reply-To: <CANq1E4TVhxDk7JvNOMQG2Qddktdp2zzRayeAhQ0Zc=vghyGw7g@mail.gmail.com>
On Sun, 17 Nov 2013, David Herrmann wrote:
> > commit 86b84167 HID: wiimote: add LEGO-wiimote VID
> >
> > introduced a VID/PID conflict with its original owner: hid-wiimote got
> > hid:b0005g*v0000054Cp00000306 added but hid-sony already has this id for
> > the PS3 Remote.
> >
> > I think the original and legitimate owner should remain the default for this
> > VID/PID combination.
>
> Thanks for the report! It's indeed officially allocated as USB ID to Sony.
>
> @Jiri, can you revert the following commit (note: it was already
> backported to stable):
>
> commit 86b84167d4e67372376a57ea9955c5d53dae232f
> Author: David Herrmann <dh.herrmann@gmail.com>
> Date: Fri Oct 18 16:26:22 2013 +0200
>
> HID: wiimote: add LEGO-wiimote VID
>
> I will write a bluez-patch to force the official wiimote-id on the
> device, based on a name-comparison. This should allow us to support
> both devices.
Thanks a lot for the report. I have queued the revert for 3.13.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox