Add a driver for the Sony PS5 DualSense controller (054c:0ce6)
providing gamepad input via evdev, lightbar RGB LED control, and
player indicator LEDs through sysctl.
Signed-off-by: Christos Longros <chris.longros@gmail.com>
Details
Details
Tested on 16.0-CURRENT/amd64 with 054c:0ce6 over USB: all buttons and both sticks and triggers through evdev, lightbar and all five player LED patterns through sysctl, detach and reattach.
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 72130 Build 69013: arc lint + arc unit
Event Timeline
| sys/dev/hid/ps5dualsense.c | ||
|---|---|---|
| 30 | you can delete this line. | |
| sys/dev/hid/ps5dualsense.c | ||
|---|---|---|
| 134 | Add Output report of 64 bytes total to HID report descriptor. It is used to properly size of USB buffers. | |
| 232 | should be PS5DS_MAP_ABS(). It does not return to central position being released at least on PS4 | |
| 233 | Ditto | |
| 234 | should be PS5DS_MAP_FLT(). It does return to central position being released at least on PS4 | |
| 235 | Ditto | |
| 236 | For PS4 it is processed by hgame_hat_switch_cb() rather than directly mapped to ABS_HAT0X | |
| 250 | This event belongs to touchpad device rather than game controller | |
| 251 | May be it should be KEY_MUTE? It is better to consult with Linux driver to find out which device should produce which event after the button is pressed. | |
| 430 | That is a race with sysctl handlers. Move this line in front of SYSCTL_ADD_PROCs or protect it with sx lock | |
This comment was removed by chris.longros_gmail.com.
Comment Actions
Looks mostly good.
There are some missing parts. TODO:
- Add manpage. You may take share/man/man4/ps4dshock.4 as starting point. Do not forget to add missing touchpad and accelerometer support to BUGS section.
- Add ps5dualsense.c to sys/conf/files and sys/conf/NOTES
- Add ps5dualsense subdir to sys/modules/hid/Makefile
| sys/dev/hid/ps5dualsense.c | ||
|---|---|---|
| 129 | Not a bug, but Usage (0x23) is better here as it matches hardware report descriptor. | |
Comment Actions
IMO it would be good to rename driver from ps5dualsense to ps5dsense to be inline with ps4dshock. That is up to you.
| sys/dev/hid/ps5dualsense.c | ||
|---|---|---|
| 129 | I have just glanced at https://controllers.fandom.com/wiki/Sony_DualSense#HID_Report_0x02_Output_USB and found out that Report 1 (input) size is 63 bytes and Report 2 (output) is 47 bytes not including leading ReportID byte. IMO both ps5ds_rdesc[] and PS5DS_OUTPUT_REPORT_USB_SIZE should be corrected to match this values. | |
Comment Actions
Rebase on current main, rename to ps5dsense, describe the full 63 byte input report, and add the manpage, sys/conf and module Makefile entries. Tested on hardware.
This comment was removed by chris.longros_gmail.com.
| share/man/man4/ps5dsense.4 | ||
|---|---|---|
| 31–47 ↗ | (On Diff #184090) | See hgame(4) |
| 76 ↗ | (On Diff #184090) | suggestion: %d is the most traditional. we should probably standardize this |
| 78 ↗ | (On Diff #184090) | |
| 81 ↗ | (On Diff #184090) | |
| 84 ↗ | (On Diff #184090) | |
| 87 ↗ | (On Diff #184090) | |
| 97 ↗ | (On Diff #184090) | Here's what we did in hgame(4). I would like to sync up all the controller manuals with the improvements I've made there. At any rate, this should not have a period because it is not a sentence. |
| 100 ↗ | (On Diff #184090) | This uses usbhid? and its configured with devfs.rules? Good for the doc to show the usage. |
| 112 ↗ | (On Diff #184090) | I really appreciate your documenting of known limitations in the BUGS section. This is exactly what it is meant for and this can really accelerate development and lead to rational expectations in the ecosystem. |
| 1–23 ↗ | (On Diff #183725) | We updated the preferred license for new files a few years ago. |
| share/man/man4/ps5dsense.4 | ||
|---|---|---|
| 112 ↗ | (On Diff #184090) | Thanks for your review ! |
| share/man/man4/ps5dsense.4 | ||
|---|---|---|
| 18–30 ↗ | (On Diff #185412) | Oh dude, I accidentally led you astray by messing up the synopsis in hgame. |
| 131–135 ↗ | (On Diff #185412) | Suggestion: Traditionally we did it this way, since you wrote everything there's no need to split them out. This is good for mechanical parsing as well. |
| sys/dev/hid/ps5dsense.c | ||
| 1 ↗ | (On Diff #185412) | If it's okay with @wulf, consider using the updated license notation here as well. |