diff mbox series

[11/13] HID: playstation: add DualShock4 bluetooth support.

Message ID 20221029184851.282366-12-roderick.colenbrander@sony.com (mailing list archive)
State Mainlined
Commit 2d77474a239294786feb4fe9864c845fe92db239
Delegated to: Jiri Kosina
Headers show
Series hid: playstation: add DualShock4 support | expand

Commit Message

Roderick Colenbrander Oct. 29, 2022, 6:48 p.m. UTC
Add support for DualShock4 in Bluetooth mode. In Bluetooth, the device
is a bit strange in that after 'calibration' it switches sending all its
input data from a basic report (only containing buttons/sticks) to an
extended report, which also contains touchpad, motion sensors and other
data. The overall design of this code is similar to the DualSense code.

Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
---
 drivers/hid/hid-playstation.c | 170 ++++++++++++++++++++++++++++------
 1 file changed, 144 insertions(+), 26 deletions(-)

Comments

Benjamin Tissoires Nov. 14, 2022, 1:33 p.m. UTC | #1
On Sat, Oct 29, 2022 at 8:49 PM Roderick Colenbrander <roderick@gaikai.com> wrote:
>
> Add support for DualShock4 in Bluetooth mode. In Bluetooth, the device
> is a bit strange in that after 'calibration' it switches sending all its
> input data from a basic report (only containing buttons/sticks) to an
> extended report, which also contains touchpad, motion sensors and other
> data. The overall design of this code is similar to the DualSense code.
>
> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> ---


Hi roderick,

Starting with this commit, the hid-tools testsuite is segfaulting:

---
hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_creation
PASSED [ 70%]
hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_buttons
PASSED [ 70%]
hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_dual_buttons
PASSED [ 70%]
hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_left_joystick_press_left
PASSED [ 70%]
hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_left_joystick_press_right
PASSED [ 70%][  534.255759] BUG: kernel NULL pointer dereference, address: 0000000000000010
[  534.256406] #PF: supervisor read access in kernel mode
[  534.256923] #PF: error_code(0x0000) - not-present page
[  534.257345] PGD 0 P4D 0
[  534.257558] Oops: 0000 [#1] PREEMPT SMP PTI
[  534.257897] CPU: 0 PID: 619 Comm: pytest-3 Not tainted 6.1.0-rc1-CI-PIPELINE-4044-g0f01ce929234 #1
[  534.258630] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-1.fc36 04/01/2014
[  534.259314] RIP: 0010:devres_release_group+0x69/0x160
[  534.259714] Code: 49 8b bc 24 98 02 00 00 49 8d b4 24 90 02 00 00 49 89 c7 48 39 fe 75 12 e9 de 00 00 00 48 8b 7f 08 48 39 fe 0f 84 d1 00 00 00 <48> 81 7f 10 f0 da fb 83 75 e9 48 85 db 0f 84 a0 00 00 00 48 3b 5f
[  534.261197] RSP: 0018:ffffa145c0887ca8 EFLAGS: 00010082
[  534.261618] RAX: 0000000000000246 RBX: ffffffff84153600 RCX: 0000000000000000
[  534.262179] RDX: 0000000000000001 RSI: ffff970980ba56c8 RDI: 0000000000000000
[  534.262731] RBP: ffff970980ba56c4 R08: 0000000000000001 R09: ffffffff83ad9800
[  534.263290] R10: 000000000000001f R11: ffff97098a9186d8 R12: ffff970980ba5438
[  534.263835] R13: ffffa145c0887ca8 R14: 0000000000000014 R15: 0000000000000246
[  534.264654] FS:  00007f3aae1db740(0000) GS:ffff9709bbc00000(0000) knlGS:0000000000000000
[  534.265584] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  534.266265] CR2: 0000000000000010 CR3: 0000000100b4c001 CR4: 0000000000170ef0
[  534.267022] Call Trace:
[  534.267215]  <TASK>
[  534.267391]  power_supply_unregister+0x55/0xc0
[  534.267770]  devres_release_all+0xb3/0x100
[  534.268100]  device_unbind_cleanup+0x9/0x70
[  534.268430]  device_release_driver_internal+0x1da/0x230
[  534.268866]  bus_remove_device+0xcd/0x110
[  534.269317]  device_del+0x18c/0x4b0
[  534.269719]  ? __cancel_work_timer+0xf5/0x190
[  534.270167]  hid_destroy_device+0x3d/0x50
[  534.270548]  uhid_char_write+0x490/0x540
[  534.270978]  vfs_write+0xc2/0x400
[  534.271349]  ? kvm_clock_get_cycles+0x14/0x30
[  534.271802]  ? ktime_get+0x36/0x90
[  534.272072]  ? lapic_timer_set_periodic+0x20/0x20
[  534.272492]  ? clockevents_program_event+0x90/0xf0
[  534.272942]  ksys_write+0xb2/0xe0
[  534.273221]  do_syscall_64+0x3a/0x90
[  534.273578]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
[  534.274073] RIP: 0033:0x7f3aae3bd8f7
[  534.274404] Code: 0f 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
[  534.276478] RSP: 002b:00007ffcbc210348 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[  534.277366] RAX: ffffffffffffffda RBX: 00007f3aae1db6c0 RCX: 00007f3aae3bd8f7
[  534.278210] RDX: 0000000000000004 RSI: 00007f3aa49b2de0 RDI: 000000000000000b
[  534.279086] RBP: 0000000000000004 R08: 0000000000000000 R09: 0000000000000000
[  534.279969] R10: 0000000000000000 R11: 0000000000000246 R12: 00007f3aa49b2de0
[  534.280816] R13: 000000000000000b R14: 0000555a22edb620 R15: 00007f3aac4dc548
[  534.281533]  </TASK>
[  534.281760] Modules linked in:
[  534.282084] CR2: 0000000000000010
[  534.282404] ---[ end trace 0000000000000000 ]---
[  534.282838] RIP: 0010:devres_release_group+0x69/0x160
[  534.283301] Code: 49 8b bc 24 98 02 00 00 49 8d b4 24 90 02 00 00 49 89 c7 48 39 fe 75 12 e9 de 00 00 00 48 8b 7f 08 48 39 fe 0f 84 d1 00 00 00 <48> 81 7f 10 f0 da fb 83 75 e9 48 85 db 0f 84 a0 00 00 00 48 3b 5f
[  534.285303] RSP: 0018:ffffa145c0887ca8 EFLAGS: 00010082
[  534.285722] RAX: 0000000000000246 RBX: ffffffff84153600 RCX: 0000000000000000
[  534.286292] RDX: 0000000000000001 RSI: ffff970980ba56c8 RDI: 0000000000000000
[  534.286885] RBP: ffff970980ba56c4 R08: 0000000000000001 R09: ffffffff83ad9800
[  534.287629] R10: 000000000000001f R11: ffff97098a9186d8 R12: ffff970980ba5438
[  534.288415] R13: ffffa145c0887ca8 R14: 0000000000000014 R15: 0000000000000246
[  534.289247] FS:  00007f3aae1db740(0000) GS:ffff9709bbc00000(0000) knlGS:0000000000000000
[  534.290184] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  534.290848] CR2: 0000000000000010 CR3: 0000000100b4c001 CR4: 0000000000170ef0
[  534.291588] note: pytest-3[619] exited with preempt_count 1
---

Would you mind having a look at it?

Cheers,
Benjamin

  >
>  drivers/hid/hid-playstation.c | 170 ++++++++++++++++++++++++++++------
>  1 file changed, 144 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index 81a12aafed17..a2f1bd1400a2 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -287,11 +287,17 @@ struct dualsense_output_report {
>
>  #define DS4_INPUT_REPORT_USB                   0x01
>  #define DS4_INPUT_REPORT_USB_SIZE              64
> +#define DS4_INPUT_REPORT_BT                    0x11
> +#define DS4_INPUT_REPORT_BT_SIZE               78
>  #define DS4_OUTPUT_REPORT_USB                  0x05
>  #define DS4_OUTPUT_REPORT_USB_SIZE             32
> +#define DS4_OUTPUT_REPORT_BT                   0x11
> +#define DS4_OUTPUT_REPORT_BT_SIZE              78
>
>  #define DS4_FEATURE_REPORT_CALIBRATION         0x02
>  #define DS4_FEATURE_REPORT_CALIBRATION_SIZE    37
> +#define DS4_FEATURE_REPORT_CALIBRATION_BT      0x05
> +#define DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE 41
>  #define DS4_FEATURE_REPORT_FIRMWARE_INFO       0xa3
>  #define DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE  49
>  #define DS4_FEATURE_REPORT_PAIRING_INFO                0x12
> @@ -310,6 +316,9 @@ struct dualsense_output_report {
>  /* Battery status within batery_status field. */
>  #define DS4_BATTERY_STATUS_FULL                11
>
> +#define DS4_OUTPUT_HWCTL_CRC32         0x40
> +#define DS4_OUTPUT_HWCTL_HID           0x80
> +
>  /* Flags for DualShock4 output report. */
>  #define DS4_OUTPUT_VALID_FLAG0_MOTOR           0x01
>  #define DS4_OUTPUT_VALID_FLAG0_LED             0x02
> @@ -401,6 +410,17 @@ struct dualshock4_input_report_usb {
>  } __packed;
>  static_assert(sizeof(struct dualshock4_input_report_usb) == DS4_INPUT_REPORT_USB_SIZE);
>
> +struct dualshock4_input_report_bt {
> +       uint8_t report_id; /* 0x11 */
> +       uint8_t reserved[2];
> +       struct dualshock4_input_report_common common;
> +       uint8_t num_touch_reports;
> +       struct dualshock4_touch_report touch_reports[4]; /* BT has 4 compared to 3 for USB */
> +       uint8_t reserved2[2];
> +       __le32 crc32;
> +} __packed;
> +static_assert(sizeof(struct dualshock4_input_report_bt) == DS4_INPUT_REPORT_BT_SIZE);
> +
>  /* Common data between Bluetooth and USB DualShock4 output reports. */
>  struct dualshock4_output_report_common {
>         uint8_t valid_flag0;
> @@ -425,6 +445,16 @@ struct dualshock4_output_report_usb {
>  } __packed;
>  static_assert(sizeof(struct dualshock4_output_report_usb) == DS4_OUTPUT_REPORT_USB_SIZE);
>
> +struct dualshock4_output_report_bt {
> +       uint8_t report_id; /* 0x11 */
> +       uint8_t hw_control;
> +       uint8_t audio_control;
> +       struct dualshock4_output_report_common common;
> +       uint8_t reserved[61];
> +       __le32 crc32;
> +} __packed;
> +static_assert(sizeof(struct dualshock4_output_report_bt) == DS4_OUTPUT_REPORT_BT_SIZE);
> +
>  /*
>   * The DualShock4 has a main output report used to control most features. It is
>   * largely the same between Bluetooth and USB except for different headers and CRC.
> @@ -434,6 +464,8 @@ struct dualshock4_output_report {
>         uint8_t *data; /* Start of data */
>         uint8_t len; /* Size of output report */
>
> +       /* Points to Bluetooth data payload in case for a Bluetooth report else NULL. */
> +       struct dualshock4_output_report_bt *bt;
>         /* Points to USB data payload in case for a USB report else NULL. */
>         struct dualshock4_output_report_usb *usb;
>         /* Points to common section of report, so past any headers. */
> @@ -1646,26 +1678,49 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
>         int ret = 0;
>         uint8_t *buf;
>
> -       buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL);
> -       if (!buf)
> -               return -ENOMEM;
> +       if (ds4->base.hdev->bus == BUS_USB) {
> +               buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL);
> +               if (!buf)
> +                       return -ENOMEM;
>
> -       ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION, buf,
> -                       DS4_FEATURE_REPORT_CALIBRATION_SIZE, true);
> -       if (ret) {
> -               hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
> -               goto err_free;
> +               ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION, buf,
> +                               DS4_FEATURE_REPORT_CALIBRATION_SIZE, true);
> +               if (ret) {
> +                       hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
> +                       goto err_free;
> +               }
> +       } else { /* Bluetooth */
> +               buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE, GFP_KERNEL);
> +               if (!buf)
> +                       return -ENOMEM;
> +
> +               ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION_BT, buf,
> +                               DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE, true);
> +               if (ret) {
> +                       hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
> +                       goto err_free;
> +               }
>         }
>
>         gyro_pitch_bias  = get_unaligned_le16(&buf[1]);
>         gyro_yaw_bias    = get_unaligned_le16(&buf[3]);
>         gyro_roll_bias   = get_unaligned_le16(&buf[5]);
> -       gyro_pitch_plus  = get_unaligned_le16(&buf[7]);
> -       gyro_pitch_minus = get_unaligned_le16(&buf[9]);
> -       gyro_yaw_plus    = get_unaligned_le16(&buf[11]);
> -       gyro_yaw_minus   = get_unaligned_le16(&buf[13]);
> -       gyro_roll_plus   = get_unaligned_le16(&buf[15]);
> -       gyro_roll_minus  = get_unaligned_le16(&buf[17]);
> +       if (ds4->base.hdev->bus == BUS_USB) {
> +               gyro_pitch_plus  = get_unaligned_le16(&buf[7]);
> +               gyro_pitch_minus = get_unaligned_le16(&buf[9]);
> +               gyro_yaw_plus    = get_unaligned_le16(&buf[11]);
> +               gyro_yaw_minus   = get_unaligned_le16(&buf[13]);
> +               gyro_roll_plus   = get_unaligned_le16(&buf[15]);
> +               gyro_roll_minus  = get_unaligned_le16(&buf[17]);
> +       } else {
> +               /* BT + Dongle */
> +               gyro_pitch_plus  = get_unaligned_le16(&buf[7]);
> +               gyro_yaw_plus    = get_unaligned_le16(&buf[9]);
> +               gyro_roll_plus   = get_unaligned_le16(&buf[11]);
> +               gyro_pitch_minus = get_unaligned_le16(&buf[13]);
> +               gyro_yaw_minus   = get_unaligned_le16(&buf[15]);
> +               gyro_roll_minus  = get_unaligned_le16(&buf[17]);
> +       }
>         gyro_speed_plus  = get_unaligned_le16(&buf[19]);
>         gyro_speed_minus = get_unaligned_le16(&buf[21]);
>         acc_x_plus       = get_unaligned_le16(&buf[23]);
> @@ -1731,8 +1786,11 @@ static int dualshock4_get_firmware_info(struct dualshock4 *ds4)
>         if (!buf)
>                 return -ENOMEM;
>
> +       /* Note USB and BT support the same feature report, but this report
> +        * lacks CRC support, so must be disabled in ps_get_report.
> +        */
>         ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_FIRMWARE_INFO, buf,
> -                       DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE, true);
> +                       DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE, false);
>         if (ret) {
>                 hid_err(ds4->base.hdev, "Failed to retrieve DualShock4 firmware info: %d\n", ret);
>                 goto err_free;
> @@ -1748,21 +1806,38 @@ static int dualshock4_get_firmware_info(struct dualshock4 *ds4)
>
>  static int dualshock4_get_mac_address(struct dualshock4 *ds4)
>  {
> +       struct hid_device *hdev = ds4->base.hdev;
>         uint8_t *buf;
>         int ret = 0;
>
> -       buf = kzalloc(DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, GFP_KERNEL);
> -       if (!buf)
> -               return -ENOMEM;
> +       if (hdev->bus == BUS_USB) {
> +               buf = kzalloc(DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, GFP_KERNEL);
> +               if (!buf)
> +                       return -ENOMEM;
> +
> +               ret = ps_get_report(hdev, DS4_FEATURE_REPORT_PAIRING_INFO, buf,
> +                               DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, false);
> +               if (ret) {
> +                       hid_err(hdev, "Failed to retrieve DualShock4 pairing info: %d\n", ret);
> +                       goto err_free;
> +               }
>
> -       ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_PAIRING_INFO, buf,
> -                       DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, true);
> -       if (ret) {
> -               hid_err(ds4->base.hdev, "Failed to retrieve DualShock4 pairing info: %d\n", ret);
> -               goto err_free;
> -       }
> +               memcpy(ds4->base.mac_address, &buf[1], sizeof(ds4->base.mac_address));
> +       } else {
> +               /* Rely on HIDP for Bluetooth */
> +               if (strlen(hdev->uniq) != 17)
> +                       return -EINVAL;
> +
> +               ret = sscanf(hdev->uniq, "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
> +                               &ds4->base.mac_address[5], &ds4->base.mac_address[4],
> +                               &ds4->base.mac_address[3], &ds4->base.mac_address[2],
> +                               &ds4->base.mac_address[1], &ds4->base.mac_address[0]);
>
> -       memcpy(ds4->base.mac_address, &buf[1], sizeof(ds4->base.mac_address));
> +               if (ret != sizeof(ds4->base.mac_address))
> +                       return -EINVAL;
> +
> +               ret = 0;
> +       }
>
>  err_free:
>         kfree(buf);
> @@ -1859,7 +1934,18 @@ static void dualshock4_init_output_report(struct dualshock4 *ds4,
>  {
>         struct hid_device *hdev = ds4->base.hdev;
>
> -       if (hdev->bus == BUS_USB) {
> +       if (hdev->bus == BUS_BLUETOOTH) {
> +               struct dualshock4_output_report_bt *bt = buf;
> +
> +               memset(bt, 0, sizeof(*bt));
> +               bt->report_id = DS4_OUTPUT_REPORT_BT;
> +
> +               rp->data = buf;
> +               rp->len = sizeof(*bt);
> +               rp->bt = bt;
> +               rp->usb = NULL;
> +               rp->common = &bt->common;
> +       } else { /* USB */
>                 struct dualshock4_output_report_usb *usb = buf;
>
>                 memset(usb, 0, sizeof(*usb));
> @@ -1867,6 +1953,7 @@ static void dualshock4_init_output_report(struct dualshock4 *ds4,
>
>                 rp->data = buf;
>                 rp->len = sizeof(*usb);
> +               rp->bt = NULL;
>                 rp->usb = usb;
>                 rp->common = &usb->common;
>         }
> @@ -1913,6 +2000,22 @@ static void dualshock4_output_worker(struct work_struct *work)
>
>         spin_unlock_irqrestore(&ds4->base.lock, flags);
>
> +       /* Bluetooth packets need additional flags as well as a CRC in the last 4 bytes. */
> +       if (report.bt) {
> +               uint32_t crc;
> +               uint8_t seed = PS_OUTPUT_CRC32_SEED;
> +
> +               /* Hardware control flags need to set to let the device know
> +                * there is HID data as well as CRC.
> +                */
> +               report.bt->hw_control = DS4_OUTPUT_HWCTL_HID | DS4_OUTPUT_HWCTL_CRC32;
> +
> +               crc = crc32_le(0xFFFFFFFF, &seed, 1);
> +               crc = ~crc32_le(crc, report.data, report.len - 4);
> +
> +               report.bt->crc32 = cpu_to_le32(crc);
> +       }
> +
>         hid_hw_output_report(ds4->base.hdev, report.data, report.len);
>  }
>
> @@ -1940,6 +2043,19 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
>                 ds4_report = &usb->common;
>                 num_touch_reports = usb->num_touch_reports;
>                 touch_reports = usb->touch_reports;
> +       } else if (hdev->bus == BUS_BLUETOOTH && report->id == DS4_INPUT_REPORT_BT &&
> +                       size == DS4_INPUT_REPORT_BT_SIZE) {
> +               struct dualshock4_input_report_bt *bt = (struct dualshock4_input_report_bt *)data;
> +
> +               /* Last 4 bytes of input report contains CRC. */
> +               if (!ps_check_crc32(PS_INPUT_CRC32_SEED, data, size - 4, bt->crc32)) {
> +                       hid_err(hdev, "DualShock4 input CRC's check failed\n");
> +                       return -EILSEQ;
> +               }
> +
> +               ds4_report = &bt->common;
> +               num_touch_reports = bt->num_touch_reports;
> +               touch_reports = bt->touch_reports;
>         } else {
>                 hid_err(hdev, "Unhandled reportID=%d\n", report->id);
>                 return -1;
> @@ -2354,7 +2470,9 @@ static void ps_remove(struct hid_device *hdev)
>
>  static const struct hid_device_id ps_devices[] = {
>         /* Sony DualShock 4 controllers for PS4 */
> +       { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER) },
> +       { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) },
>         /* Sony DualSense controllers for PS5 */
>         { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) },
> --
> 2.37.3
>
Roderick Colenbrander Nov. 14, 2022, 4:53 p.m. UTC | #2
On Mon, Nov 14, 2022 at 5:37 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
>
>
> On Sat, Oct 29, 2022 at 8:49 PM Roderick Colenbrander <roderick@gaikai.com> wrote:
> >
> > Add support for DualShock4 in Bluetooth mode. In Bluetooth, the device
> > is a bit strange in that after 'calibration' it switches sending all its
> > input data from a basic report (only containing buttons/sticks) to an
> > extended report, which also contains touchpad, motion sensors and other
> > data. The overall design of this code is similar to the DualSense code.
> >
> > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> > ---
>
>
> Hi roderick,
>
> Starting with this commit, the hid-tools testsuite is segfaulting:
>
> ---
> hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_creation
> PASSED [ 70%]
> hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_buttons
> PASSED [ 70%]
> hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_dual_buttons
> PASSED [ 70%]
> hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_left_joystick_press_left
> PASSED [ 70%]
> hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_left_joystick_press_right
> PASSED [ 70%][  534.255759] BUG: kernel NULL pointer dereference, address: 0000000000000010
> [  534.256406] #PF: supervisor read access in kernel mode
> [  534.256923] #PF: error_code(0x0000) - not-present page
> [  534.257345] PGD 0 P4D 0
> [  534.257558] Oops: 0000 [#1] PREEMPT SMP PTI
> [  534.257897] CPU: 0 PID: 619 Comm: pytest-3 Not tainted 6.1.0-rc1-CI-PIPELINE-4044-g0f01ce929234 #1
> [  534.258630] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-1.fc36 04/01/2014
> [  534.259314] RIP: 0010:devres_release_group+0x69/0x160
> [  534.259714] Code: 49 8b bc 24 98 02 00 00 49 8d b4 24 90 02 00 00 49 89 c7 48 39 fe 75 12 e9 de 00 00 00 48 8b 7f 08 48 39 fe 0f 84 d1 00 00 00 <48> 81 7f 10 f0 da fb 83 75 e9 48 85 db 0f 84 a0 00 00 00 48 3b 5f
> [  534.261197] RSP: 0018:ffffa145c0887ca8 EFLAGS: 00010082
> [  534.261618] RAX: 0000000000000246 RBX: ffffffff84153600 RCX: 0000000000000000
> [  534.262179] RDX: 0000000000000001 RSI: ffff970980ba56c8 RDI: 0000000000000000
> [  534.262731] RBP: ffff970980ba56c4 R08: 0000000000000001 R09: ffffffff83ad9800
> [  534.263290] R10: 000000000000001f R11: ffff97098a9186d8 R12: ffff970980ba5438
> [  534.263835] R13: ffffa145c0887ca8 R14: 0000000000000014 R15: 0000000000000246
> [  534.264654] FS:  00007f3aae1db740(0000) GS:ffff9709bbc00000(0000) knlGS:0000000000000000
> [  534.265584] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  534.266265] CR2: 0000000000000010 CR3: 0000000100b4c001 CR4: 0000000000170ef0
> [  534.267022] Call Trace:
> [  534.267215]  <TASK>
> [  534.267391]  power_supply_unregister+0x55/0xc0
> [  534.267770]  devres_release_all+0xb3/0x100
> [  534.268100]  device_unbind_cleanup+0x9/0x70
> [  534.268430]  device_release_driver_internal+0x1da/0x230
> [  534.268866]  bus_remove_device+0xcd/0x110
> [  534.269317]  device_del+0x18c/0x4b0
> [  534.269719]  ? __cancel_work_timer+0xf5/0x190
> [  534.270167]  hid_destroy_device+0x3d/0x50
> [  534.270548]  uhid_char_write+0x490/0x540
> [  534.270978]  vfs_write+0xc2/0x400
> [  534.271349]  ? kvm_clock_get_cycles+0x14/0x30
> [  534.271802]  ? ktime_get+0x36/0x90
> [  534.272072]  ? lapic_timer_set_periodic+0x20/0x20
> [  534.272492]  ? clockevents_program_event+0x90/0xf0
> [  534.272942]  ksys_write+0xb2/0xe0
> [  534.273221]  do_syscall_64+0x3a/0x90
> [  534.273578]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
> [  534.274073] RIP: 0033:0x7f3aae3bd8f7
> [  534.274404] Code: 0f 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
> [  534.276478] RSP: 002b:00007ffcbc210348 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> [  534.277366] RAX: ffffffffffffffda RBX: 00007f3aae1db6c0 RCX: 00007f3aae3bd8f7
> [  534.278210] RDX: 0000000000000004 RSI: 00007f3aa49b2de0 RDI: 000000000000000b
> [  534.279086] RBP: 0000000000000004 R08: 0000000000000000 R09: 0000000000000000
> [  534.279969] R10: 0000000000000000 R11: 0000000000000246 R12: 00007f3aa49b2de0
> [  534.280816] R13: 000000000000000b R14: 0000555a22edb620 R15: 00007f3aac4dc548
> [  534.281533]  </TASK>
> [  534.281760] Modules linked in:
> [  534.282084] CR2: 0000000000000010
> [  534.282404] ---[ end trace 0000000000000000 ]---
> [  534.282838] RIP: 0010:devres_release_group+0x69/0x160
> [  534.283301] Code: 49 8b bc 24 98 02 00 00 49 8d b4 24 90 02 00 00 49 89 c7 48 39 fe 75 12 e9 de 00 00 00 48 8b 7f 08 48 39 fe 0f 84 d1 00 00 00 <48> 81 7f 10 f0 da fb 83 75 e9 48 85 db 0f 84 a0 00 00 00 48 3b 5f
> [  534.285303] RSP: 0018:ffffa145c0887ca8 EFLAGS: 00010082
> [  534.285722] RAX: 0000000000000246 RBX: ffffffff84153600 RCX: 0000000000000000
> [  534.286292] RDX: 0000000000000001 RSI: ffff970980ba56c8 RDI: 0000000000000000
> [  534.286885] RBP: ffff970980ba56c4 R08: 0000000000000001 R09: ffffffff83ad9800
> [  534.287629] R10: 000000000000001f R11: ffff97098a9186d8 R12: ffff970980ba5438
> [  534.288415] R13: ffffa145c0887ca8 R14: 0000000000000014 R15: 0000000000000246
> [  534.289247] FS:  00007f3aae1db740(0000) GS:ffff9709bbc00000(0000) knlGS:0000000000000000
> [  534.290184] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  534.290848] CR2: 0000000000000010 CR3: 0000000100b4c001 CR4: 0000000000170ef0
> [  534.291588] note: pytest-3[619] exited with preempt_count 1
> ---
>
> Would you mind having a look at it?
>
> Cheers,
> Benjamin
>

Fun, fun... I don't know what it is, but looks like a race condition /
memory corruption somewhere. I just saw the issue as well, but I had
to run the test suite a few times as I got some clean runs in as well.
I must have been luckily to have submitted on a clean run.

[root@localhost hid-tools]# pytest tests/test_sony.py
====================================================================
test session starts
=====================================================================
platform linux -- Python 3.10.7, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
rootdir: /home/roderick/src/hid-tools
collected 95 items

tests/test_sony.py
...........ssss................................................................................
                                    [100%]

=============================================================== 91
passed, 4 skipped in 48.68s
===============================================================

Thanks,
Roderick
Roderick Colenbrander Nov. 14, 2022, 8:14 p.m. UTC | #3
On Mon, Nov 14, 2022 at 8:53 AM Roderick Colenbrander
<thunderbird2k@gmail.com> wrote:
>
> On Mon, Nov 14, 2022 at 5:37 AM Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> >
> >
> >
> > On Sat, Oct 29, 2022 at 8:49 PM Roderick Colenbrander <roderick@gaikai.com> wrote:
> > >
> > > Add support for DualShock4 in Bluetooth mode. In Bluetooth, the device
> > > is a bit strange in that after 'calibration' it switches sending all its
> > > input data from a basic report (only containing buttons/sticks) to an
> > > extended report, which also contains touchpad, motion sensors and other
> > > data. The overall design of this code is similar to the DualSense code.
> > >
> > > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> > > ---
> >
> >
> > Hi roderick,
> >
> > Starting with this commit, the hid-tools testsuite is segfaulting:
> >
> > ---
> > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_creation
> > PASSED [ 70%]
> > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_buttons
> > PASSED [ 70%]
> > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_dual_buttons
> > PASSED [ 70%]
> > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_left_joystick_press_left
> > PASSED [ 70%]
> > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_left_joystick_press_right
> > PASSED [ 70%][  534.255759] BUG: kernel NULL pointer dereference, address: 0000000000000010
> > [  534.256406] #PF: supervisor read access in kernel mode
> > [  534.256923] #PF: error_code(0x0000) - not-present page
> > [  534.257345] PGD 0 P4D 0
> > [  534.257558] Oops: 0000 [#1] PREEMPT SMP PTI
> > [  534.257897] CPU: 0 PID: 619 Comm: pytest-3 Not tainted 6.1.0-rc1-CI-PIPELINE-4044-g0f01ce929234 #1
> > [  534.258630] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-1.fc36 04/01/2014
> > [  534.259314] RIP: 0010:devres_release_group+0x69/0x160
> > [  534.259714] Code: 49 8b bc 24 98 02 00 00 49 8d b4 24 90 02 00 00 49 89 c7 48 39 fe 75 12 e9 de 00 00 00 48 8b 7f 08 48 39 fe 0f 84 d1 00 00 00 <48> 81 7f 10 f0 da fb 83 75 e9 48 85 db 0f 84 a0 00 00 00 48 3b 5f
> > [  534.261197] RSP: 0018:ffffa145c0887ca8 EFLAGS: 00010082
> > [  534.261618] RAX: 0000000000000246 RBX: ffffffff84153600 RCX: 0000000000000000
> > [  534.262179] RDX: 0000000000000001 RSI: ffff970980ba56c8 RDI: 0000000000000000
> > [  534.262731] RBP: ffff970980ba56c4 R08: 0000000000000001 R09: ffffffff83ad9800
> > [  534.263290] R10: 000000000000001f R11: ffff97098a9186d8 R12: ffff970980ba5438
> > [  534.263835] R13: ffffa145c0887ca8 R14: 0000000000000014 R15: 0000000000000246
> > [  534.264654] FS:  00007f3aae1db740(0000) GS:ffff9709bbc00000(0000) knlGS:0000000000000000
> > [  534.265584] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [  534.266265] CR2: 0000000000000010 CR3: 0000000100b4c001 CR4: 0000000000170ef0
> > [  534.267022] Call Trace:
> > [  534.267215]  <TASK>
> > [  534.267391]  power_supply_unregister+0x55/0xc0
> > [  534.267770]  devres_release_all+0xb3/0x100
> > [  534.268100]  device_unbind_cleanup+0x9/0x70
> > [  534.268430]  device_release_driver_internal+0x1da/0x230
> > [  534.268866]  bus_remove_device+0xcd/0x110
> > [  534.269317]  device_del+0x18c/0x4b0
> > [  534.269719]  ? __cancel_work_timer+0xf5/0x190
> > [  534.270167]  hid_destroy_device+0x3d/0x50
> > [  534.270548]  uhid_char_write+0x490/0x540
> > [  534.270978]  vfs_write+0xc2/0x400
> > [  534.271349]  ? kvm_clock_get_cycles+0x14/0x30
> > [  534.271802]  ? ktime_get+0x36/0x90
> > [  534.272072]  ? lapic_timer_set_periodic+0x20/0x20
> > [  534.272492]  ? clockevents_program_event+0x90/0xf0
> > [  534.272942]  ksys_write+0xb2/0xe0
> > [  534.273221]  do_syscall_64+0x3a/0x90
> > [  534.273578]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
> > [  534.274073] RIP: 0033:0x7f3aae3bd8f7
> > [  534.274404] Code: 0f 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
> > [  534.276478] RSP: 002b:00007ffcbc210348 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> > [  534.277366] RAX: ffffffffffffffda RBX: 00007f3aae1db6c0 RCX: 00007f3aae3bd8f7
> > [  534.278210] RDX: 0000000000000004 RSI: 00007f3aa49b2de0 RDI: 000000000000000b
> > [  534.279086] RBP: 0000000000000004 R08: 0000000000000000 R09: 0000000000000000
> > [  534.279969] R10: 0000000000000000 R11: 0000000000000246 R12: 00007f3aa49b2de0
> > [  534.280816] R13: 000000000000000b R14: 0000555a22edb620 R15: 00007f3aac4dc548
> > [  534.281533]  </TASK>
> > [  534.281760] Modules linked in:
> > [  534.282084] CR2: 0000000000000010
> > [  534.282404] ---[ end trace 0000000000000000 ]---
> > [  534.282838] RIP: 0010:devres_release_group+0x69/0x160
> > [  534.283301] Code: 49 8b bc 24 98 02 00 00 49 8d b4 24 90 02 00 00 49 89 c7 48 39 fe 75 12 e9 de 00 00 00 48 8b 7f 08 48 39 fe 0f 84 d1 00 00 00 <48> 81 7f 10 f0 da fb 83 75 e9 48 85 db 0f 84 a0 00 00 00 48 3b 5f
> > [  534.285303] RSP: 0018:ffffa145c0887ca8 EFLAGS: 00010082
> > [  534.285722] RAX: 0000000000000246 RBX: ffffffff84153600 RCX: 0000000000000000
> > [  534.286292] RDX: 0000000000000001 RSI: ffff970980ba56c8 RDI: 0000000000000000
> > [  534.286885] RBP: ffff970980ba56c4 R08: 0000000000000001 R09: ffffffff83ad9800
> > [  534.287629] R10: 000000000000001f R11: ffff97098a9186d8 R12: ffff970980ba5438
> > [  534.288415] R13: ffffa145c0887ca8 R14: 0000000000000014 R15: 0000000000000246
> > [  534.289247] FS:  00007f3aae1db740(0000) GS:ffff9709bbc00000(0000) knlGS:0000000000000000
> > [  534.290184] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [  534.290848] CR2: 0000000000000010 CR3: 0000000100b4c001 CR4: 0000000000170ef0
> > [  534.291588] note: pytest-3[619] exited with preempt_count 1
> > ---
> >
> > Would you mind having a look at it?
> >
> > Cheers,
> > Benjamin
> >
>
> Fun, fun... I don't know what it is, but looks like a race condition /
> memory corruption somewhere. I just saw the issue as well, but I had
> to run the test suite a few times as I got some clean runs in as well.
> I must have been luckily to have submitted on a clean run.
>
> [root@localhost hid-tools]# pytest tests/test_sony.py
> ====================================================================
> test session starts
> =====================================================================
> platform linux -- Python 3.10.7, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
> rootdir: /home/roderick/src/hid-tools
> collected 95 items
>
> tests/test_sony.py
> ...........ssss................................................................................
>                                     [100%]
>
> =============================================================== 91
> passed, 4 skipped in 48.68s
> ===============================================================
>
> Thanks,
> Roderick

I now have it happening consistently including for a real DualShock 4
in bluetooth. What the heck. It is something devres related on
cleanup. I disabled some code paths in the driver (e.g. storing
devices in a list and player id) and then the issue jumps around to
then power_supply_unregister as called by devres.

At least I'm glad this issue surfaces now, but I'm not yet sure where
to search. I find it is so weird that Bluetooth patch shows this. USB
is fine and the changes do look fairly straight-forward with nothing
obviously sticking out yet...

Roderick
Roderick Colenbrander Nov. 15, 2022, 12:05 a.m. UTC | #4
On Mon, Nov 14, 2022 at 12:14 PM Roderick Colenbrander
<thunderbird2k@gmail.com> wrote:
>
> On Mon, Nov 14, 2022 at 8:53 AM Roderick Colenbrander
> <thunderbird2k@gmail.com> wrote:
> >
> > On Mon, Nov 14, 2022 at 5:37 AM Benjamin Tissoires
> > <benjamin.tissoires@redhat.com> wrote:
> > >
> > >
> > >
> > > On Sat, Oct 29, 2022 at 8:49 PM Roderick Colenbrander <roderick@gaikai.com> wrote:
> > > >
> > > > Add support for DualShock4 in Bluetooth mode. In Bluetooth, the device
> > > > is a bit strange in that after 'calibration' it switches sending all its
> > > > input data from a basic report (only containing buttons/sticks) to an
> > > > extended report, which also contains touchpad, motion sensors and other
> > > > data. The overall design of this code is similar to the DualSense code.
> > > >
> > > > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> > > > ---
> > >
> > >
> > > Hi roderick,
> > >
> > > Starting with this commit, the hid-tools testsuite is segfaulting:
> > >
> > > ---
> > > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_creation
> > > PASSED [ 70%]
> > > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_buttons
> > > PASSED [ 70%]
> > > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_dual_buttons
> > > PASSED [ 70%]
> > > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_left_joystick_press_left
> > > PASSED [ 70%]
> > > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_left_joystick_press_right
> > > PASSED [ 70%][  534.255759] BUG: kernel NULL pointer dereference, address: 0000000000000010
> > > [  534.256406] #PF: supervisor read access in kernel mode
> > > [  534.256923] #PF: error_code(0x0000) - not-present page
> > > [  534.257345] PGD 0 P4D 0
> > > [  534.257558] Oops: 0000 [#1] PREEMPT SMP PTI
> > > [  534.257897] CPU: 0 PID: 619 Comm: pytest-3 Not tainted 6.1.0-rc1-CI-PIPELINE-4044-g0f01ce929234 #1
> > > [  534.258630] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-1.fc36 04/01/2014
> > > [  534.259314] RIP: 0010:devres_release_group+0x69/0x160
> > > [  534.259714] Code: 49 8b bc 24 98 02 00 00 49 8d b4 24 90 02 00 00 49 89 c7 48 39 fe 75 12 e9 de 00 00 00 48 8b 7f 08 48 39 fe 0f 84 d1 00 00 00 <48> 81 7f 10 f0 da fb 83 75 e9 48 85 db 0f 84 a0 00 00 00 48 3b 5f
> > > [  534.261197] RSP: 0018:ffffa145c0887ca8 EFLAGS: 00010082
> > > [  534.261618] RAX: 0000000000000246 RBX: ffffffff84153600 RCX: 0000000000000000
> > > [  534.262179] RDX: 0000000000000001 RSI: ffff970980ba56c8 RDI: 0000000000000000
> > > [  534.262731] RBP: ffff970980ba56c4 R08: 0000000000000001 R09: ffffffff83ad9800
> > > [  534.263290] R10: 000000000000001f R11: ffff97098a9186d8 R12: ffff970980ba5438
> > > [  534.263835] R13: ffffa145c0887ca8 R14: 0000000000000014 R15: 0000000000000246
> > > [  534.264654] FS:  00007f3aae1db740(0000) GS:ffff9709bbc00000(0000) knlGS:0000000000000000
> > > [  534.265584] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > [  534.266265] CR2: 0000000000000010 CR3: 0000000100b4c001 CR4: 0000000000170ef0
> > > [  534.267022] Call Trace:
> > > [  534.267215]  <TASK>
> > > [  534.267391]  power_supply_unregister+0x55/0xc0
> > > [  534.267770]  devres_release_all+0xb3/0x100
> > > [  534.268100]  device_unbind_cleanup+0x9/0x70
> > > [  534.268430]  device_release_driver_internal+0x1da/0x230
> > > [  534.268866]  bus_remove_device+0xcd/0x110
> > > [  534.269317]  device_del+0x18c/0x4b0
> > > [  534.269719]  ? __cancel_work_timer+0xf5/0x190
> > > [  534.270167]  hid_destroy_device+0x3d/0x50
> > > [  534.270548]  uhid_char_write+0x490/0x540
> > > [  534.270978]  vfs_write+0xc2/0x400
> > > [  534.271349]  ? kvm_clock_get_cycles+0x14/0x30
> > > [  534.271802]  ? ktime_get+0x36/0x90
> > > [  534.272072]  ? lapic_timer_set_periodic+0x20/0x20
> > > [  534.272492]  ? clockevents_program_event+0x90/0xf0
> > > [  534.272942]  ksys_write+0xb2/0xe0
> > > [  534.273221]  do_syscall_64+0x3a/0x90
> > > [  534.273578]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
> > > [  534.274073] RIP: 0033:0x7f3aae3bd8f7
> > > [  534.274404] Code: 0f 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
> > > [  534.276478] RSP: 002b:00007ffcbc210348 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> > > [  534.277366] RAX: ffffffffffffffda RBX: 00007f3aae1db6c0 RCX: 00007f3aae3bd8f7
> > > [  534.278210] RDX: 0000000000000004 RSI: 00007f3aa49b2de0 RDI: 000000000000000b
> > > [  534.279086] RBP: 0000000000000004 R08: 0000000000000000 R09: 0000000000000000
> > > [  534.279969] R10: 0000000000000000 R11: 0000000000000246 R12: 00007f3aa49b2de0
> > > [  534.280816] R13: 000000000000000b R14: 0000555a22edb620 R15: 00007f3aac4dc548
> > > [  534.281533]  </TASK>
> > > [  534.281760] Modules linked in:
> > > [  534.282084] CR2: 0000000000000010
> > > [  534.282404] ---[ end trace 0000000000000000 ]---
> > > [  534.282838] RIP: 0010:devres_release_group+0x69/0x160
> > > [  534.283301] Code: 49 8b bc 24 98 02 00 00 49 8d b4 24 90 02 00 00 49 89 c7 48 39 fe 75 12 e9 de 00 00 00 48 8b 7f 08 48 39 fe 0f 84 d1 00 00 00 <48> 81 7f 10 f0 da fb 83 75 e9 48 85 db 0f 84 a0 00 00 00 48 3b 5f
> > > [  534.285303] RSP: 0018:ffffa145c0887ca8 EFLAGS: 00010082
> > > [  534.285722] RAX: 0000000000000246 RBX: ffffffff84153600 RCX: 0000000000000000
> > > [  534.286292] RDX: 0000000000000001 RSI: ffff970980ba56c8 RDI: 0000000000000000
> > > [  534.286885] RBP: ffff970980ba56c4 R08: 0000000000000001 R09: ffffffff83ad9800
> > > [  534.287629] R10: 000000000000001f R11: ffff97098a9186d8 R12: ffff970980ba5438
> > > [  534.288415] R13: ffffa145c0887ca8 R14: 0000000000000014 R15: 0000000000000246
> > > [  534.289247] FS:  00007f3aae1db740(0000) GS:ffff9709bbc00000(0000) knlGS:0000000000000000
> > > [  534.290184] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > [  534.290848] CR2: 0000000000000010 CR3: 0000000100b4c001 CR4: 0000000000170ef0
> > > [  534.291588] note: pytest-3[619] exited with preempt_count 1
> > > ---
> > >
> > > Would you mind having a look at it?
> > >
> > > Cheers,
> > > Benjamin
> > >
> >
> > Fun, fun... I don't know what it is, but looks like a race condition /
> > memory corruption somewhere. I just saw the issue as well, but I had
> > to run the test suite a few times as I got some clean runs in as well.
> > I must have been luckily to have submitted on a clean run.
> >
> > [root@localhost hid-tools]# pytest tests/test_sony.py
> > ====================================================================
> > test session starts
> > =====================================================================
> > platform linux -- Python 3.10.7, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
> > rootdir: /home/roderick/src/hid-tools
> > collected 95 items
> >
> > tests/test_sony.py
> > ...........ssss................................................................................
> >                                     [100%]
> >
> > =============================================================== 91
> > passed, 4 skipped in 48.68s
> > ===============================================================
> >
> > Thanks,
> > Roderick
>
> I now have it happening consistently including for a real DualShock 4
> in bluetooth. What the heck. It is something devres related on
> cleanup. I disabled some code paths in the driver (e.g. storing
> devices in a list and player id) and then the issue jumps around to
> then power_supply_unregister as called by devres.
>
> At least I'm glad this issue surfaces now, but I'm not yet sure where
> to search. I find it is so weird that Bluetooth patch shows this. USB
> is fine and the changes do look fairly straight-forward with nothing
> obviously sticking out yet...
>
> Roderick

I found the issue it was some memory corruption due to incorrect
output buffer size. Bluetooth needs a bigger buffer (78 vs 32 bytes),
so some data must have gotten overwritten elsewhere.

How should I submit this change? Single patch or resubmit patch 11?

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index bae3e71..f5e0d06 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -2461,7 +2461,7 @@ static struct ps_device
*dualshock4_create(struct hid_device *hdev)
        ds4->output_worker_initialized = true;
        hid_set_drvdata(hdev, ds4);

-       max_output_report_size = sizeof(struct dualshock4_output_report_usb);
+       max_output_report_size = sizeof(struct dualshock4_output_report_bt);
        ds4->output_report_dmabuf = devm_kzalloc(&hdev->dev,
max_output_report_size, GFP_KERNEL);
        if (!ds4->output_report_dmabuf)
                return ERR_PTR(-ENOMEM)
Benjamin Tissoires Nov. 15, 2022, 8:26 a.m. UTC | #5
On Tue, Nov 15, 2022 at 1:05 AM Roderick Colenbrander
<thunderbird2k@gmail.com> wrote:
>
> On Mon, Nov 14, 2022 at 12:14 PM Roderick Colenbrander
> <thunderbird2k@gmail.com> wrote:
> >
> > On Mon, Nov 14, 2022 at 8:53 AM Roderick Colenbrander
> > <thunderbird2k@gmail.com> wrote:
> > >
> > > On Mon, Nov 14, 2022 at 5:37 AM Benjamin Tissoires
> > > <benjamin.tissoires@redhat.com> wrote:
> > > >
> > > >
> > > >
> > > > On Sat, Oct 29, 2022 at 8:49 PM Roderick Colenbrander <roderick@gaikai.com> wrote:
> > > > >
> > > > > Add support for DualShock4 in Bluetooth mode. In Bluetooth, the device
> > > > > is a bit strange in that after 'calibration' it switches sending all its
> > > > > input data from a basic report (only containing buttons/sticks) to an
> > > > > extended report, which also contains touchpad, motion sensors and other
> > > > > data. The overall design of this code is similar to the DualSense code.
> > > > >
> > > > > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> > > > > ---
> > > >
> > > >
> > > > Hi roderick,
> > > >
> > > > Starting with this commit, the hid-tools testsuite is segfaulting:
> > > >
> > > > ---
> > > > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_creation
> > > > PASSED [ 70%]
> > > > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_buttons
> > > > PASSED [ 70%]
> > > > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_dual_buttons
> > > > PASSED [ 70%]
> > > > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_left_joystick_press_left
> > > > PASSED [ 70%]
> > > > hid-tools/tests/test_sony.py::TestPS4ControllerBluetooth::test_left_joystick_press_right
> > > > PASSED [ 70%][  534.255759] BUG: kernel NULL pointer dereference, address: 0000000000000010
> > > > [  534.256406] #PF: supervisor read access in kernel mode
> > > > [  534.256923] #PF: error_code(0x0000) - not-present page
> > > > [  534.257345] PGD 0 P4D 0
> > > > [  534.257558] Oops: 0000 [#1] PREEMPT SMP PTI
> > > > [  534.257897] CPU: 0 PID: 619 Comm: pytest-3 Not tainted 6.1.0-rc1-CI-PIPELINE-4044-g0f01ce929234 #1
> > > > [  534.258630] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-1.fc36 04/01/2014
> > > > [  534.259314] RIP: 0010:devres_release_group+0x69/0x160
> > > > [  534.259714] Code: 49 8b bc 24 98 02 00 00 49 8d b4 24 90 02 00 00 49 89 c7 48 39 fe 75 12 e9 de 00 00 00 48 8b 7f 08 48 39 fe 0f 84 d1 00 00 00 <48> 81 7f 10 f0 da fb 83 75 e9 48 85 db 0f 84 a0 00 00 00 48 3b 5f
> > > > [  534.261197] RSP: 0018:ffffa145c0887ca8 EFLAGS: 00010082
> > > > [  534.261618] RAX: 0000000000000246 RBX: ffffffff84153600 RCX: 0000000000000000
> > > > [  534.262179] RDX: 0000000000000001 RSI: ffff970980ba56c8 RDI: 0000000000000000
> > > > [  534.262731] RBP: ffff970980ba56c4 R08: 0000000000000001 R09: ffffffff83ad9800
> > > > [  534.263290] R10: 000000000000001f R11: ffff97098a9186d8 R12: ffff970980ba5438
> > > > [  534.263835] R13: ffffa145c0887ca8 R14: 0000000000000014 R15: 0000000000000246
> > > > [  534.264654] FS:  00007f3aae1db740(0000) GS:ffff9709bbc00000(0000) knlGS:0000000000000000
> > > > [  534.265584] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > [  534.266265] CR2: 0000000000000010 CR3: 0000000100b4c001 CR4: 0000000000170ef0
> > > > [  534.267022] Call Trace:
> > > > [  534.267215]  <TASK>
> > > > [  534.267391]  power_supply_unregister+0x55/0xc0
> > > > [  534.267770]  devres_release_all+0xb3/0x100
> > > > [  534.268100]  device_unbind_cleanup+0x9/0x70
> > > > [  534.268430]  device_release_driver_internal+0x1da/0x230
> > > > [  534.268866]  bus_remove_device+0xcd/0x110
> > > > [  534.269317]  device_del+0x18c/0x4b0
> > > > [  534.269719]  ? __cancel_work_timer+0xf5/0x190
> > > > [  534.270167]  hid_destroy_device+0x3d/0x50
> > > > [  534.270548]  uhid_char_write+0x490/0x540
> > > > [  534.270978]  vfs_write+0xc2/0x400
> > > > [  534.271349]  ? kvm_clock_get_cycles+0x14/0x30
> > > > [  534.271802]  ? ktime_get+0x36/0x90
> > > > [  534.272072]  ? lapic_timer_set_periodic+0x20/0x20
> > > > [  534.272492]  ? clockevents_program_event+0x90/0xf0
> > > > [  534.272942]  ksys_write+0xb2/0xe0
> > > > [  534.273221]  do_syscall_64+0x3a/0x90
> > > > [  534.273578]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
> > > > [  534.274073] RIP: 0033:0x7f3aae3bd8f7
> > > > [  534.274404] Code: 0f 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
> > > > [  534.276478] RSP: 002b:00007ffcbc210348 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> > > > [  534.277366] RAX: ffffffffffffffda RBX: 00007f3aae1db6c0 RCX: 00007f3aae3bd8f7
> > > > [  534.278210] RDX: 0000000000000004 RSI: 00007f3aa49b2de0 RDI: 000000000000000b
> > > > [  534.279086] RBP: 0000000000000004 R08: 0000000000000000 R09: 0000000000000000
> > > > [  534.279969] R10: 0000000000000000 R11: 0000000000000246 R12: 00007f3aa49b2de0
> > > > [  534.280816] R13: 000000000000000b R14: 0000555a22edb620 R15: 00007f3aac4dc548
> > > > [  534.281533]  </TASK>
> > > > [  534.281760] Modules linked in:
> > > > [  534.282084] CR2: 0000000000000010
> > > > [  534.282404] ---[ end trace 0000000000000000 ]---
> > > > [  534.282838] RIP: 0010:devres_release_group+0x69/0x160
> > > > [  534.283301] Code: 49 8b bc 24 98 02 00 00 49 8d b4 24 90 02 00 00 49 89 c7 48 39 fe 75 12 e9 de 00 00 00 48 8b 7f 08 48 39 fe 0f 84 d1 00 00 00 <48> 81 7f 10 f0 da fb 83 75 e9 48 85 db 0f 84 a0 00 00 00 48 3b 5f
> > > > [  534.285303] RSP: 0018:ffffa145c0887ca8 EFLAGS: 00010082
> > > > [  534.285722] RAX: 0000000000000246 RBX: ffffffff84153600 RCX: 0000000000000000
> > > > [  534.286292] RDX: 0000000000000001 RSI: ffff970980ba56c8 RDI: 0000000000000000
> > > > [  534.286885] RBP: ffff970980ba56c4 R08: 0000000000000001 R09: ffffffff83ad9800
> > > > [  534.287629] R10: 000000000000001f R11: ffff97098a9186d8 R12: ffff970980ba5438
> > > > [  534.288415] R13: ffffa145c0887ca8 R14: 0000000000000014 R15: 0000000000000246
> > > > [  534.289247] FS:  00007f3aae1db740(0000) GS:ffff9709bbc00000(0000) knlGS:0000000000000000
> > > > [  534.290184] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > [  534.290848] CR2: 0000000000000010 CR3: 0000000100b4c001 CR4: 0000000000170ef0
> > > > [  534.291588] note: pytest-3[619] exited with preempt_count 1
> > > > ---
> > > >
> > > > Would you mind having a look at it?
> > > >
> > > > Cheers,
> > > > Benjamin
> > > >
> > >
> > > Fun, fun... I don't know what it is, but looks like a race condition /
> > > memory corruption somewhere. I just saw the issue as well, but I had
> > > to run the test suite a few times as I got some clean runs in as well.
> > > I must have been luckily to have submitted on a clean run.
> > >
> > > [root@localhost hid-tools]# pytest tests/test_sony.py
> > > ====================================================================
> > > test session starts
> > > =====================================================================
> > > platform linux -- Python 3.10.7, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
> > > rootdir: /home/roderick/src/hid-tools
> > > collected 95 items
> > >
> > > tests/test_sony.py
> > > ...........ssss................................................................................
> > >                                     [100%]
> > >
> > > =============================================================== 91
> > > passed, 4 skipped in 48.68s
> > > ===============================================================
> > >
> > > Thanks,
> > > Roderick
> >
> > I now have it happening consistently including for a real DualShock 4
> > in bluetooth. What the heck. It is something devres related on
> > cleanup. I disabled some code paths in the driver (e.g. storing
> > devices in a list and player id) and then the issue jumps around to
> > then power_supply_unregister as called by devres.
> >
> > At least I'm glad this issue surfaces now, but I'm not yet sure where
> > to search. I find it is so weird that Bluetooth patch shows this. USB
> > is fine and the changes do look fairly straight-forward with nothing
> > obviously sticking out yet...
> >
> > Roderick
>
> I found the issue it was some memory corruption due to incorrect
> output buffer size. Bluetooth needs a bigger buffer (78 vs 32 bytes),
> so some data must have gotten overwritten elsewhere.

Great, thanks a lot for the fix :)

>
> How should I submit this change? Single patch or resubmit patch 11?

Please submit a fix on top of for-6.2/sony [0] with the proper fixes tag.

Cheers,
Benjamin


[0] https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-6.2/sony

>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index bae3e71..f5e0d06 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -2461,7 +2461,7 @@ static struct ps_device
> *dualshock4_create(struct hid_device *hdev)
>         ds4->output_worker_initialized = true;
>         hid_set_drvdata(hdev, ds4);
>
> -       max_output_report_size = sizeof(struct dualshock4_output_report_usb);
> +       max_output_report_size = sizeof(struct dualshock4_output_report_bt);
>         ds4->output_report_dmabuf = devm_kzalloc(&hdev->dev,
> max_output_report_size, GFP_KERNEL);
>         if (!ds4->output_report_dmabuf)
>                 return ERR_PTR(-ENOMEM)
>
diff mbox series

Patch

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 81a12aafed17..a2f1bd1400a2 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -287,11 +287,17 @@  struct dualsense_output_report {
 
 #define DS4_INPUT_REPORT_USB			0x01
 #define DS4_INPUT_REPORT_USB_SIZE		64
+#define DS4_INPUT_REPORT_BT			0x11
+#define DS4_INPUT_REPORT_BT_SIZE		78
 #define DS4_OUTPUT_REPORT_USB			0x05
 #define DS4_OUTPUT_REPORT_USB_SIZE		32
+#define DS4_OUTPUT_REPORT_BT			0x11
+#define DS4_OUTPUT_REPORT_BT_SIZE		78
 
 #define DS4_FEATURE_REPORT_CALIBRATION		0x02
 #define DS4_FEATURE_REPORT_CALIBRATION_SIZE	37
+#define DS4_FEATURE_REPORT_CALIBRATION_BT	0x05
+#define DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE	41
 #define DS4_FEATURE_REPORT_FIRMWARE_INFO	0xa3
 #define DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE	49
 #define DS4_FEATURE_REPORT_PAIRING_INFO		0x12
@@ -310,6 +316,9 @@  struct dualsense_output_report {
 /* Battery status within batery_status field. */
 #define DS4_BATTERY_STATUS_FULL		11
 
+#define DS4_OUTPUT_HWCTL_CRC32		0x40
+#define DS4_OUTPUT_HWCTL_HID		0x80
+
 /* Flags for DualShock4 output report. */
 #define DS4_OUTPUT_VALID_FLAG0_MOTOR		0x01
 #define DS4_OUTPUT_VALID_FLAG0_LED		0x02
@@ -401,6 +410,17 @@  struct dualshock4_input_report_usb {
 } __packed;
 static_assert(sizeof(struct dualshock4_input_report_usb) == DS4_INPUT_REPORT_USB_SIZE);
 
+struct dualshock4_input_report_bt {
+	uint8_t report_id; /* 0x11 */
+	uint8_t reserved[2];
+	struct dualshock4_input_report_common common;
+	uint8_t num_touch_reports;
+	struct dualshock4_touch_report touch_reports[4]; /* BT has 4 compared to 3 for USB */
+	uint8_t reserved2[2];
+	__le32 crc32;
+} __packed;
+static_assert(sizeof(struct dualshock4_input_report_bt) == DS4_INPUT_REPORT_BT_SIZE);
+
 /* Common data between Bluetooth and USB DualShock4 output reports. */
 struct dualshock4_output_report_common {
 	uint8_t valid_flag0;
@@ -425,6 +445,16 @@  struct dualshock4_output_report_usb {
 } __packed;
 static_assert(sizeof(struct dualshock4_output_report_usb) == DS4_OUTPUT_REPORT_USB_SIZE);
 
+struct dualshock4_output_report_bt {
+	uint8_t report_id; /* 0x11 */
+	uint8_t hw_control;
+	uint8_t audio_control;
+	struct dualshock4_output_report_common common;
+	uint8_t reserved[61];
+	__le32 crc32;
+} __packed;
+static_assert(sizeof(struct dualshock4_output_report_bt) == DS4_OUTPUT_REPORT_BT_SIZE);
+
 /*
  * The DualShock4 has a main output report used to control most features. It is
  * largely the same between Bluetooth and USB except for different headers and CRC.
@@ -434,6 +464,8 @@  struct dualshock4_output_report {
 	uint8_t *data; /* Start of data */
 	uint8_t len; /* Size of output report */
 
+	/* Points to Bluetooth data payload in case for a Bluetooth report else NULL. */
+	struct dualshock4_output_report_bt *bt;
 	/* Points to USB data payload in case for a USB report else NULL. */
 	struct dualshock4_output_report_usb *usb;
 	/* Points to common section of report, so past any headers. */
@@ -1646,26 +1678,49 @@  static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 	int ret = 0;
 	uint8_t *buf;
 
-	buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
+	if (ds4->base.hdev->bus == BUS_USB) {
+		buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL);
+		if (!buf)
+			return -ENOMEM;
 
-	ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION, buf,
-			DS4_FEATURE_REPORT_CALIBRATION_SIZE, true);
-	if (ret) {
-		hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
-		goto err_free;
+		ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION, buf,
+				DS4_FEATURE_REPORT_CALIBRATION_SIZE, true);
+		if (ret) {
+			hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
+			goto err_free;
+		}
+	} else { /* Bluetooth */
+		buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE, GFP_KERNEL);
+		if (!buf)
+			return -ENOMEM;
+
+		ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION_BT, buf,
+				DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE, true);
+		if (ret) {
+			hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
+			goto err_free;
+		}
 	}
 
 	gyro_pitch_bias  = get_unaligned_le16(&buf[1]);
 	gyro_yaw_bias    = get_unaligned_le16(&buf[3]);
 	gyro_roll_bias   = get_unaligned_le16(&buf[5]);
-	gyro_pitch_plus  = get_unaligned_le16(&buf[7]);
-	gyro_pitch_minus = get_unaligned_le16(&buf[9]);
-	gyro_yaw_plus    = get_unaligned_le16(&buf[11]);
-	gyro_yaw_minus   = get_unaligned_le16(&buf[13]);
-	gyro_roll_plus   = get_unaligned_le16(&buf[15]);
-	gyro_roll_minus  = get_unaligned_le16(&buf[17]);
+	if (ds4->base.hdev->bus == BUS_USB) {
+		gyro_pitch_plus  = get_unaligned_le16(&buf[7]);
+		gyro_pitch_minus = get_unaligned_le16(&buf[9]);
+		gyro_yaw_plus    = get_unaligned_le16(&buf[11]);
+		gyro_yaw_minus   = get_unaligned_le16(&buf[13]);
+		gyro_roll_plus   = get_unaligned_le16(&buf[15]);
+		gyro_roll_minus  = get_unaligned_le16(&buf[17]);
+	} else {
+		/* BT + Dongle */
+		gyro_pitch_plus  = get_unaligned_le16(&buf[7]);
+		gyro_yaw_plus    = get_unaligned_le16(&buf[9]);
+		gyro_roll_plus   = get_unaligned_le16(&buf[11]);
+		gyro_pitch_minus = get_unaligned_le16(&buf[13]);
+		gyro_yaw_minus   = get_unaligned_le16(&buf[15]);
+		gyro_roll_minus  = get_unaligned_le16(&buf[17]);
+	}
 	gyro_speed_plus  = get_unaligned_le16(&buf[19]);
 	gyro_speed_minus = get_unaligned_le16(&buf[21]);
 	acc_x_plus       = get_unaligned_le16(&buf[23]);
@@ -1731,8 +1786,11 @@  static int dualshock4_get_firmware_info(struct dualshock4 *ds4)
 	if (!buf)
 		return -ENOMEM;
 
+	/* Note USB and BT support the same feature report, but this report
+	 * lacks CRC support, so must be disabled in ps_get_report.
+	 */
 	ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_FIRMWARE_INFO, buf,
-			DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE, true);
+			DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE, false);
 	if (ret) {
 		hid_err(ds4->base.hdev, "Failed to retrieve DualShock4 firmware info: %d\n", ret);
 		goto err_free;
@@ -1748,21 +1806,38 @@  static int dualshock4_get_firmware_info(struct dualshock4 *ds4)
 
 static int dualshock4_get_mac_address(struct dualshock4 *ds4)
 {
+	struct hid_device *hdev = ds4->base.hdev;
 	uint8_t *buf;
 	int ret = 0;
 
-	buf = kzalloc(DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
+	if (hdev->bus == BUS_USB) {
+		buf = kzalloc(DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, GFP_KERNEL);
+		if (!buf)
+			return -ENOMEM;
+
+		ret = ps_get_report(hdev, DS4_FEATURE_REPORT_PAIRING_INFO, buf,
+				DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, false);
+		if (ret) {
+			hid_err(hdev, "Failed to retrieve DualShock4 pairing info: %d\n", ret);
+			goto err_free;
+		}
 
-	ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_PAIRING_INFO, buf,
-			DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, true);
-	if (ret) {
-		hid_err(ds4->base.hdev, "Failed to retrieve DualShock4 pairing info: %d\n", ret);
-		goto err_free;
-	}
+		memcpy(ds4->base.mac_address, &buf[1], sizeof(ds4->base.mac_address));
+	} else {
+		/* Rely on HIDP for Bluetooth */
+		if (strlen(hdev->uniq) != 17)
+			return -EINVAL;
+
+		ret = sscanf(hdev->uniq, "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
+				&ds4->base.mac_address[5], &ds4->base.mac_address[4],
+				&ds4->base.mac_address[3], &ds4->base.mac_address[2],
+				&ds4->base.mac_address[1], &ds4->base.mac_address[0]);
 
-	memcpy(ds4->base.mac_address, &buf[1], sizeof(ds4->base.mac_address));
+		if (ret != sizeof(ds4->base.mac_address))
+			return -EINVAL;
+
+		ret = 0;
+	}
 
 err_free:
 	kfree(buf);
@@ -1859,7 +1934,18 @@  static void dualshock4_init_output_report(struct dualshock4 *ds4,
 {
 	struct hid_device *hdev = ds4->base.hdev;
 
-	if (hdev->bus == BUS_USB) {
+	if (hdev->bus == BUS_BLUETOOTH) {
+		struct dualshock4_output_report_bt *bt = buf;
+
+		memset(bt, 0, sizeof(*bt));
+		bt->report_id = DS4_OUTPUT_REPORT_BT;
+
+		rp->data = buf;
+		rp->len = sizeof(*bt);
+		rp->bt = bt;
+		rp->usb = NULL;
+		rp->common = &bt->common;
+	} else { /* USB */
 		struct dualshock4_output_report_usb *usb = buf;
 
 		memset(usb, 0, sizeof(*usb));
@@ -1867,6 +1953,7 @@  static void dualshock4_init_output_report(struct dualshock4 *ds4,
 
 		rp->data = buf;
 		rp->len = sizeof(*usb);
+		rp->bt = NULL;
 		rp->usb = usb;
 		rp->common = &usb->common;
 	}
@@ -1913,6 +2000,22 @@  static void dualshock4_output_worker(struct work_struct *work)
 
 	spin_unlock_irqrestore(&ds4->base.lock, flags);
 
+	/* Bluetooth packets need additional flags as well as a CRC in the last 4 bytes. */
+	if (report.bt) {
+		uint32_t crc;
+		uint8_t seed = PS_OUTPUT_CRC32_SEED;
+
+		/* Hardware control flags need to set to let the device know
+		 * there is HID data as well as CRC.
+		 */
+		report.bt->hw_control = DS4_OUTPUT_HWCTL_HID | DS4_OUTPUT_HWCTL_CRC32;
+
+		crc = crc32_le(0xFFFFFFFF, &seed, 1);
+		crc = ~crc32_le(crc, report.data, report.len - 4);
+
+		report.bt->crc32 = cpu_to_le32(crc);
+	}
+
 	hid_hw_output_report(ds4->base.hdev, report.data, report.len);
 }
 
@@ -1940,6 +2043,19 @@  static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 		ds4_report = &usb->common;
 		num_touch_reports = usb->num_touch_reports;
 		touch_reports = usb->touch_reports;
+	} else if (hdev->bus == BUS_BLUETOOTH && report->id == DS4_INPUT_REPORT_BT &&
+			size == DS4_INPUT_REPORT_BT_SIZE) {
+		struct dualshock4_input_report_bt *bt = (struct dualshock4_input_report_bt *)data;
+
+		/* Last 4 bytes of input report contains CRC. */
+		if (!ps_check_crc32(PS_INPUT_CRC32_SEED, data, size - 4, bt->crc32)) {
+			hid_err(hdev, "DualShock4 input CRC's check failed\n");
+			return -EILSEQ;
+		}
+
+		ds4_report = &bt->common;
+		num_touch_reports = bt->num_touch_reports;
+		touch_reports = bt->touch_reports;
 	} else {
 		hid_err(hdev, "Unhandled reportID=%d\n", report->id);
 		return -1;
@@ -2354,7 +2470,9 @@  static void ps_remove(struct hid_device *hdev)
 
 static const struct hid_device_id ps_devices[] = {
 	/* Sony DualShock 4 controllers for PS4 */
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) },
 	/* Sony DualSense controllers for PS5 */
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) },