| Message ID | alpine.DEB.2.10.1401031518010.29517@franz-virtual-machine (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
> This patch adds force feedback support for Sony's Dualshock 4 controller. > It does this by adding a device ID for the new controller and creating a > new dualshock4_worker function to send data commands formatted for the > controller. Unlike the Sixaxis, the Dualshock 4 requires a magnitude > value for the small motor so the actual value is now sent to the worker > function and the sixaxis worker was modified to clamp it to 1 or 0 as > required. I was able to build this and the LED patch against the 3.13rc6 kernel having first applied this sequence of patches: https://patchwork.kernel.org/patch/3203761/ However I am seeing some weird behaviour in whether FF/LED actually functions. It seems that in a 'complete' kernel installation the driver does not present FF or LED. But if I use a stock 3.13rc6 kernel, load the 'hid-sony' module from that and then remove and install the patched one it does work. -- $ sudo modprobe hid-sony (stock) $ sudo rmmod hid-sony $ sudo insmod hid-sony.ko (patched) -- Not sure what this means, lsmod looks to be the same in both. Perhaps there is something which is only initialised in the stock version. Willing to test if anyone has suggestions/comments. I am testing with the DS4 connected with USB (not BT). -- Jan 4 17:55:14 simon-virtual-machine kernel: [ 138.964626] usb 1-1: new full-speed USB device number 3 using ohci-pci Jan 4 17:55:14 simon-virtual-machine mtp-probe: checking bus 1, device 3: "/sys/devices/pci0000:00/0000:00:06.0/usb1/1-1" Jan 4 17:55:14 simon-virtual-machine mtp-probe: bus: 1, device: 3 was not an MTP device Jan 4 17:55:14 simon-virtual-machine kernel: [ 139.416656] usb 1-1: New USB device found, idVendor=054c, idProduct=05c4 Jan 4 17:55:14 simon-virtual-machine kernel: [ 139.416661] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0 Jan 4 17:55:14 simon-virtual-machine kernel: [ 139.416663] usb 1-1: Product: Wireless Controller Jan 4 17:55:14 simon-virtual-machine kernel: [ 139.416665] usb 1-1: Manufacturer: Sony Computer Entertainment Jan 4 17:55:14 simon-virtual-machine kernel: [ 139.517356] input: Sony Computer Entertainment Wireless Controller as /devices/pci0000:00/0000:00:06.0/usb1/1-1/1-1:1.0/input/input6 Jan 4 17:55:14 simon-virtual-machine kernel: [ 139.518359] hid-generic 0003:054C:05C4.0002: input,hidraw0: USB HID v1.11 Gamepad [Sony Computer Entertainment Wireless Controller] on usb-0000:00:06.0-1/input0 -- simon@simon-virtual-machine:~$ ls /sys/class/leds/ simon@simon-virtual-machine:~$ -- Cheers, Simon -- 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
> However I am seeing some weird behaviour in whether FF/LED actually > functions. It seems that in a 'complete' kernel installation the driver > does not present FF or LED. > > But if I use a stock 3.13rc6 kernel, load the 'hid-sony' module from that > and then remove and install the patched one it does work. > -- > $ sudo modprobe hid-sony (stock) > $ sudo rmmod hid-sony > $ sudo insmod hid-sony.ko (patched) > -- I note that using the above modprobe/rmmod/insmod trick the message log shows additional lines -- Jan 8 20:12:29 atom kernel: [ 144.840112] input: Sony Computer Entertainment Wireless Controller as /devices/pci0000:00/0000:00:1d.1/usb3/3-1/3-1:1.0/input/input9 Jan 8 20:12:29 atom kernel: [ 144.841044] sony 0003:054C:05C4.0001: input,hidraw0: USB HID v1.11 Gamepad [Sony Computer Entertainment Wireless Controller] on usb-0000:00:1d.1-1/input0 Jan 8 20:12:29 atom kernel: [ 144.841560] usbcore: registered new interface driver usbhid Jan 8 20:12:29 atom kernel: [ 144.841563] usbhid: USB HID core driver -- and the Sony driver is being used -- root@atom:/sys/bus/usb/devices/3-1:1.0# ls -al 0003\:054C\:05C4.0001/ total 0 drwxr-xr-x 5 root root 0 Jan 8 20:12 . drwxr-xr-x 7 root root 0 Jan 8 20:12 .. lrwxrwxrwx 1 root root 0 Jan 8 20:13 driver -> ../../../../../../../bus/hid/drivers/sony -- Whereas with the fully patched kernel it is not -- root@atom:/sys/bus/usb/devices/3-1:1.0# ls -al 0003\:054C\:05C4.0001/ total 0 drwxr-xr-x 4 root root 0 Jan 8 20:20 . drwxr-xr-x 7 root root 0 Jan 8 20:20 .. lrwxrwxrwx 1 root root 0 Jan 8 20:21 driver -> ../../../../../../../bus/hid/drivers/hid-generic -- Further more my DS3-SixAxis appears to work OK with the full kernel, where the DS4 does not.... any suggestions? Simon -- 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
> >> However I am seeing some weird behaviour in whether FF/LED actually >> functions. It seems that in a 'complete' kernel installation the driver >> does not present FF or LED. >> Found the problem (or at least a solution), there is no entry in 'hid-core.c' -- static const struct hid_device_id hid_have_special_driver[] = { ... { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER) }, -- Adding this makes it work for me, although I don't know why the insmod trick was working.... Simon -- 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
On Fri, 3 Jan 2014 16:03:10 -0500 (EST) Frank Praznik <frank.praznik@oh.rr.com> wrote: Hi Frank, just some notes about the style. > This patch adds force feedback support for Sony's Dualshock 4 controller. You can just use the imperative form in the long commit message too: "Add force feedback support for Sony's Dualshock 4 controller..." No need to repeat "This patch ..." in the commit message. > It does this by adding a device ID for the new controller and creating a > new dualshock4_worker function to send data commands formatted for the > controller. Unlike the Sixaxis, the Dualshock 4 requires a magnitude > value for the small motor so the actual value is now sent to the worker > function and the sixaxis worker was modified to clamp it to 1 or 0 as required. > > This patch is built against the for-3.14/sony branch in jikos/hid.git and > is compatible with the recent fixes. this last sentence is more like an annotation, put annotations after the three dashes below, just before the diffstat, don't put them in the commit message because they will become outdated and they will look odd when someone reads the project history at some point in the future. An alternative is to generate a cover-letter with git-format-patch and mention there the prerequisites for the patch series. > Signed-off-by Frank Praznik <frank.praznik@oh.rr.com> > > --- > drivers/hid/hid-ids.h | 1 + > drivers/hid/hid-sony.c | 41 +++++++++++++++++++++++++++++++++++++---- > 2 files changed, 38 insertions(+), 4 deletions(-) > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > index 60336f06..ebbd292 100644 > --- a/drivers/hid/hid-ids.h > +++ b/drivers/hid/hid-ids.h > @@ -769,6 +769,7 @@ > #define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f > #define USB_DEVICE_ID_SONY_BUZZ_CONTROLLER 0x0002 > #define USB_DEVICE_ID_SONY_WIRELESS_BUZZ_CONTROLLER 0x1000 > +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER 0x05c4 > You can align the value with the other ones using two TABs in this case. > #define USB_VENDOR_ID_SOUNDGRAPH 0x15c2 > #define USB_DEVICE_ID_SOUNDGRAPH_IMON_FIRST 0x0034 > diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c > index f57ab5e..f42c866 100644 > --- a/drivers/hid/hid-sony.c > +++ b/drivers/hid/hid-sony.c > @@ -38,6 +38,7 @@ > #define SIXAXIS_CONTROLLER_BT BIT(2) > #define BUZZ_CONTROLLER BIT(3) > #define PS3REMOTE BIT(4) > +#define DUALSHOCK4_CONTROLLER BIT(5\) most of the entries are aligned with spaces here so use spaces, PS3REMOTE will be fixed eventually. Also, what about the backslash before the parenthesis? > #define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER) > > @@ -615,7 +616,7 @@ error_leds: > return ret; > } > > -static void sony_state_worker(struct work_struct *work) > +static void sixaxis_state_worker(struct work_struct *work) this is a rename, I understand it is in preparation for dualshock4_state_worker() but it's not _directly_ related to the topic of the patch; split cleanups and cosmetic changes from functional changes, it is easier to review and validate logically contained patches. > { > struct sony_sc *sc = container_of(work, struct sony_sc, state_worker); > unsigned char buf[] = { > @@ -630,7 +631,7 @@ static void sony_state_worker(struct work_struct *work) > }; > > #ifdef CONFIG_SONY_FF > - buf[3] = sc->right; > + buf[3] = sc->right ? 1 : 0; > buf[5] = sc->left; > #endif > > @@ -640,6 +641,29 @@ static void sony_state_worker(struct work_struct *work) > HID_OUTPUT_REPORT); > } > > +static void dualshock4_state_worker(struct work_struct *work) > +{ > + struct sony_sc *sc = container_of(work, struct sony_sc, state_worker); > + unsigned char buf[] = { > + 0x05, > + 0x03, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, > + }; > + > +#ifdef CONFIG_SONY_FF > + buf[4] = sc->right; > + buf[5] = sc->left; > +#endif > + > + 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) > @@ -651,7 +675,7 @@ static int sony_play_effect(struct input_dev *dev, void *data, > return 0; > > sc->left = effect->u.rumble.strong_magnitude / 256; > - sc->right = effect->u.rumble.weak_magnitude ? 1 : 0; > + sc->right = effect->u.rumble.weak_magnitude / 256; > > schedule_work(&sc->state_worker); > return 0; > @@ -724,10 +748,14 @@ 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); > + INIT_WORK(&sc->state_worker, sixaxis_state_worker); > } > else if (sc->quirks & SIXAXIS_CONTROLLER_BT) > ret = sixaxis_set_operational_bt(hdev); > + else if (sc->quirks & DUALSHOCK4_CONTROLLER) { > + ret = 0; > + INIT_WORK(&sc->state_worker, dualshock4_state_worker); > + } > else > ret = 0; > > @@ -787,6 +815,11 @@ static const struct hid_device_id sony_devices[] = { > /* Logitech Harmony Adapter for PS3 */ > { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3), > .driver_data = PS3REMOTE }, > + /* Sony Dualshock 4 controllers for PS4 */ > + { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER), > + .driver_data = DUALSHOCK4_CONTROLLER }, > + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER), > + .driver_data = DUALSHOCK4_CONTROLLER }, > { } > }; > MODULE_DEVICE_TABLE(hid, sony_devices); > -- > 1.8.3.2 > > -- > 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
On 1/9/2014 01:42, simon@mungewell.org wrote: >>> However I am seeing some weird behaviour in whether FF/LED actually >>> functions. It seems that in a 'complete' kernel installation the driver >>> does not present FF or LED. >>> > Found the problem (or at least a solution), there is no entry in 'hid-core.c' > -- > static const struct hid_device_id hid_have_special_driver[] = { > ... > { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, > USB_DEVICE_ID_SONY_PS4_CONTROLLER) }, > -- > > Adding this makes it work for me, although I don't know why the insmod > trick was working.... > Simon > Ah, I didn't realize that it needed an entry there. I'll add this to v2 of the patch set. Thanks! -- 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
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 60336f06..ebbd292 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -769,6 +769,7 @@ #define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f #define USB_DEVICE_ID_SONY_BUZZ_CONTROLLER 0x0002 #define USB_DEVICE_ID_SONY_WIRELESS_BUZZ_CONTROLLER 0x1000 +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER 0x05c4 #define USB_VENDOR_ID_SOUNDGRAPH 0x15c2 #define USB_DEVICE_ID_SOUNDGRAPH_IMON_FIRST 0x0034 diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c index f57ab5e..f42c866 100644 --- a/drivers/hid/hid-sony.c +++ b/drivers/hid/hid-sony.c @@ -38,6 +38,7 @@ #define SIXAXIS_CONTROLLER_BT BIT(2) #define BUZZ_CONTROLLER BIT(3) #define PS3REMOTE BIT(4) +#define DUALSHOCK4_CONTROLLER BIT(5) #define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER) @@ -615,7 +616,7 @@ error_leds: return ret; } -static void sony_state_worker(struct work_struct *work) +static void sixaxis_state_worker(struct work_struct *work) { struct sony_sc *sc = container_of(work, struct sony_sc, state_worker); unsigned char buf[] = { @@ -630,7 +631,7 @@ static void sony_state_worker(struct work_struct *work) }; #ifdef CONFIG_SONY_FF - buf[3] = sc->right; + buf[3] = sc->right ? 1 : 0; buf[5] = sc->left; #endif @@ -640,6 +641,29 @@ static void sony_state_worker(struct work_struct *work) HID_OUTPUT_REPORT); } +static void dualshock4_state_worker(struct work_struct *work) +{ + struct sony_sc *sc = container_of(work, struct sony_sc, state_worker); + unsigned char buf[] = { + 0x05, + 0x03, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, + }; + +#ifdef CONFIG_SONY_FF + buf[4] = sc->right; + buf[5] = sc->left; +#endif + + 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) @@ -651,7 +675,7 @@ static int sony_play_effect(struct input_dev *dev, void *data, return 0; sc->left = effect->u.rumble.strong_magnitude / 256; - sc->right = effect->u.rumble.weak_magnitude ? 1 : 0; + sc->right = effect->u.rumble.weak_magnitude / 256; schedule_work(&sc->state_worker); return 0; @@ -724,10 +748,14 @@ 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); + INIT_WORK(&sc->state_worker, sixaxis_state_worker); } else if (sc->quirks & SIXAXIS_CONTROLLER_BT) ret = sixaxis_set_operational_bt(hdev); + else if (sc->quirks & DUALSHOCK4_CONTROLLER) { + ret = 0; + INIT_WORK(&sc->state_worker, dualshock4_state_worker); + } else ret = 0; @@ -787,6 +815,11 @@ static const struct hid_device_id sony_devices[] = { /* Logitech Harmony Adapter for PS3 */ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3), .driver_data = PS3REMOTE }, + /* Sony Dualshock 4 controllers for PS4 */ + { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER), + .driver_data = DUALSHOCK4_CONTROLLER }, + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER), + .driver_data = DUALSHOCK4_CONTROLLER }, { } }; MODULE_DEVICE_TABLE(hid, sony_devices);