[PATCH 00/13] hid: playstation: add DualShock4 support

[PATCH 00/13] hid: playstation: add DualShock4 support

[Roderick Colenbrander]

Hi,

Last year, we introduced hid-playstation as the start of a new driver
for officially supported PlayStation peripherals. The driver initially
only supported DualSense devices, but we promised to also bring support
for at least DualShock4 as well.

This patch series adds DualShock4 support to hid-playstation. It should
be considered a brand new driver written from scratch in the same design
as hid-playstation. The driver is more documented and uses structures
instead of byte offsets with magical values. The driver should be more
clear and easier to follow. A few little sections of code cary over, which
Sony contributed before for sensor calibration or dongle support.

Functionality wise the driver is equivalent to hid-sony. The only subtle
change is in the naming of the lightbar LEDs. Previously they used a naming
scheme like '<mac_address>:<color>", which doesn't follow the LED class standards.
The new scheme uses '<device_name>:<color>' (e.g. input10:red), which is more
compliant. Due to backwards compatibility in particular for Android, we couldn't
make it fully compliant. Nor were we able to use the new multicolor LED class.

Aside from the LED code, the other features behave the same way. The hid-tools
tests all pass as well. One small change is that we use a different HID report
0x12 to get the MAC address in USB mode. This report is the official one even
though other ones can get the info too, but e.g. clone devices don't tend to
support it.

Thanks,
Roderick Colenbrander
Sony Interactive Entertainment, LLC

Roderick Colenbrander (13):
  HID: playstation: initial DualShock4 USB support.
  HID: playstation: report DualShock4 hardware and firmware version.
  HID: playstation: add DualShock4 battery support.
  HID: playstation: add DualShock4 touchpad support.
  HID: playstation: add DualShock4 accelerometer and gyroscope support.
  HID: playstation: Add DualShock4 rumble support.
  HID: playstation: make LED brightness adjustable in ps_led_register.
  HID: playstation: support DualShock4 lightbar.
  HID: playstation: support DualShock4 lightbar blink.
  HID: playstation: add option to ignore CRC in ps_get_report.
  HID: playstation: add DualShock4 bluetooth support.
  HID: playstation: set default DualShock4 BT poll interval to 4ms.
  HID: playstation: add DualShock4 dongle support.

 drivers/hid/hid-playstation.c | 1135 ++++++++++++++++++++++++++++++++-
 1 file changed, 1120 insertions(+), 15 deletions(-)

-- 
2.37.3

[PATCH 01/13] HID: playstation: initial DualShock4 USB support.

[Roderick Colenbrander]

Add basic support for DualShock4 USB controller with buttons and sticks.

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

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 0b58763bfd30..04d03d2e40db 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -2,7 +2,7 @@
 /*
  *  HID driver for Sony DualSense(TM) controller.
  *
- *  Copyright (c) 2020 Sony Interactive Entertainment
+ *  Copyright (c) 2020-2022 Sony Interactive Entertainment
  */
 
 #include <linux/bits.h>
@@ -283,6 +283,35 @@ struct dualsense_output_report {
 	struct dualsense_output_report_common *common;
 };
 
+#define DS4_INPUT_REPORT_USB			0x01
+#define DS4_INPUT_REPORT_USB_SIZE		64
+
+#define DS4_FEATURE_REPORT_PAIRING_INFO		0x12
+#define DS4_FEATURE_REPORT_PAIRING_INFO_SIZE	16
+
+struct dualshock4 {
+	struct ps_device base;
+	struct input_dev *gamepad;
+};
+
+/* Main DualShock4 input report excluding any BT/USB specific headers. */
+struct dualshock4_input_report_common {
+	uint8_t x, y;
+	uint8_t rx, ry;
+	uint8_t buttons[3];
+	uint8_t z, rz;
+
+	uint8_t reserved[20];
+} __packed;
+static_assert(sizeof(struct dualshock4_input_report_common) == 29);
+
+struct dualshock4_input_report_usb {
+	uint8_t report_id; /* 0x01 */
+	struct dualshock4_input_report_common common;
+	uint8_t reserved[34];
+} __packed;
+static_assert(sizeof(struct dualshock4_input_report_usb) == DS4_INPUT_REPORT_USB_SIZE);
+
 /*
  * Common gamepad buttons across DualShock 3 / 4 and DualSense.
  * Note: for device with a touchpad, touchpad button is not included
@@ -1465,6 +1494,135 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
 	return ERR_PTR(ret);
 }
 
+static int dualshock4_get_mac_address(struct dualshock4 *ds4)
+{
+	uint8_t *buf;
+	int ret = 0;
+
+	buf = kzalloc(DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_PAIRING_INFO, buf,
+			DS4_FEATURE_REPORT_PAIRING_INFO_SIZE);
+	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));
+
+err_free:
+	kfree(buf);
+	return ret;
+}
+
+static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *report,
+		u8 *data, int size)
+{
+	struct hid_device *hdev = ps_dev->hdev;
+	struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
+	struct dualshock4_input_report_common *ds4_report;
+	uint8_t value;
+
+	/*
+	 * DualShock4 in USB uses the full HID report for reportID 1, but
+	 * Bluetooth uses a minimal HID report for reportID 1 and reports
+	 * the full report using reportID 17.
+	 */
+	if (hdev->bus == BUS_USB && report->id == DS4_INPUT_REPORT_USB &&
+			size == DS4_INPUT_REPORT_USB_SIZE) {
+		struct dualshock4_input_report_usb *usb = (struct dualshock4_input_report_usb *)data;
+
+		ds4_report = &usb->common;
+	} else {
+		hid_err(hdev, "Unhandled reportID=%d\n", report->id);
+		return -1;
+	}
+
+	input_report_abs(ds4->gamepad, ABS_X,  ds4_report->x);
+	input_report_abs(ds4->gamepad, ABS_Y,  ds4_report->y);
+	input_report_abs(ds4->gamepad, ABS_RX, ds4_report->rx);
+	input_report_abs(ds4->gamepad, ABS_RY, ds4_report->ry);
+	input_report_abs(ds4->gamepad, ABS_Z,  ds4_report->z);
+	input_report_abs(ds4->gamepad, ABS_RZ, ds4_report->rz);
+
+	value = ds4_report->buttons[0] & DS_BUTTONS0_HAT_SWITCH;
+	if (value >= ARRAY_SIZE(ps_gamepad_hat_mapping))
+		value = 8; /* center */
+	input_report_abs(ds4->gamepad, ABS_HAT0X, ps_gamepad_hat_mapping[value].x);
+	input_report_abs(ds4->gamepad, ABS_HAT0Y, ps_gamepad_hat_mapping[value].y);
+
+	input_report_key(ds4->gamepad, BTN_WEST,   ds4_report->buttons[0] & DS_BUTTONS0_SQUARE);
+	input_report_key(ds4->gamepad, BTN_SOUTH,  ds4_report->buttons[0] & DS_BUTTONS0_CROSS);
+	input_report_key(ds4->gamepad, BTN_EAST,   ds4_report->buttons[0] & DS_BUTTONS0_CIRCLE);
+	input_report_key(ds4->gamepad, BTN_NORTH,  ds4_report->buttons[0] & DS_BUTTONS0_TRIANGLE);
+	input_report_key(ds4->gamepad, BTN_TL,     ds4_report->buttons[1] & DS_BUTTONS1_L1);
+	input_report_key(ds4->gamepad, BTN_TR,     ds4_report->buttons[1] & DS_BUTTONS1_R1);
+	input_report_key(ds4->gamepad, BTN_TL2,    ds4_report->buttons[1] & DS_BUTTONS1_L2);
+	input_report_key(ds4->gamepad, BTN_TR2,    ds4_report->buttons[1] & DS_BUTTONS1_R2);
+	input_report_key(ds4->gamepad, BTN_SELECT, ds4_report->buttons[1] & DS_BUTTONS1_CREATE);
+	input_report_key(ds4->gamepad, BTN_START,  ds4_report->buttons[1] & DS_BUTTONS1_OPTIONS);
+	input_report_key(ds4->gamepad, BTN_THUMBL, ds4_report->buttons[1] & DS_BUTTONS1_L3);
+	input_report_key(ds4->gamepad, BTN_THUMBR, ds4_report->buttons[1] & DS_BUTTONS1_R3);
+	input_report_key(ds4->gamepad, BTN_MODE,   ds4_report->buttons[2] & DS_BUTTONS2_PS_HOME);
+	input_sync(ds4->gamepad);
+
+	return 0;
+}
+
+static struct ps_device *dualshock4_create(struct hid_device *hdev)
+{
+	struct dualshock4 *ds4;
+	struct ps_device *ps_dev;
+	int ret;
+
+	ds4 = devm_kzalloc(&hdev->dev, sizeof(*ds4), GFP_KERNEL);
+	if (!ds4)
+		return ERR_PTR(-ENOMEM);
+
+	/*
+	 * Patch version to allow userspace to distinguish between
+	 * hid-generic vs hid-playstation axis and button mapping.
+	 */
+	hdev->version |= HID_PLAYSTATION_VERSION_PATCH;
+
+	ps_dev = &ds4->base;
+	ps_dev->hdev = hdev;
+	spin_lock_init(&ps_dev->lock);
+	ps_dev->parse_report = dualshock4_parse_report;
+	hid_set_drvdata(hdev, ds4);
+
+	ret = dualshock4_get_mac_address(ds4);
+	if (ret) {
+		hid_err(hdev, "Failed to get MAC address from DualShock4\n");
+		return ERR_PTR(ret);
+	}
+	snprintf(hdev->uniq, sizeof(hdev->uniq), "%pMR", ds4->base.mac_address);
+
+	ret = ps_devices_list_add(ps_dev);
+	if (ret)
+		return ERR_PTR(ret);
+
+	ds4->gamepad = ps_gamepad_create(hdev, NULL);
+	if (IS_ERR(ds4->gamepad)) {
+		ret = PTR_ERR(ds4->gamepad);
+		goto err;
+	}
+
+	ret = ps_device_set_player_id(ps_dev);
+	if (ret) {
+		hid_err(hdev, "Failed to assign player id for DualShock4: %d\n", ret);
+		goto err;
+	}
+
+	return &ds4->base;
+
+err:
+	ps_devices_list_remove(ps_dev);
+	return ERR_PTR(ret);
+}
+
 static int ps_raw_event(struct hid_device *hdev, struct hid_report *report,
 		u8 *data, int size)
 {
@@ -1499,7 +1657,15 @@ static int ps_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_stop;
 	}
 
-	if (hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER ||
+	if (hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER ||
+		hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) {
+		dev = dualshock4_create(hdev);
+		if (IS_ERR(dev)) {
+			hid_err(hdev, "Failed to create dualshock4.\n");
+			ret = PTR_ERR(dev);
+			goto err_close;
+		}
+	} else if (hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER ||
 		hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER_2) {
 		dev = dualsense_create(hdev);
 		if (IS_ERR(dev)) {
@@ -1533,6 +1699,10 @@ static void ps_remove(struct hid_device *hdev)
 }
 
 static const struct hid_device_id ps_devices[] = {
+	/* Sony DualShock 4 controllers for PS4 */
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER) },
+	{ 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) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) },
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER_2) },
-- 
2.37.3

[PATCH 02/13] HID: playstation: report DualShock4 hardware and firmware version.

[Roderick Colenbrander]

Report DualShock4 hardware and firmware version info through sysfs.
It uses the same sysfs nodes as the DualSense did (and hid-sony).

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

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 04d03d2e40db..df4353a4f1e9 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -286,6 +286,8 @@ struct dualsense_output_report {
 #define DS4_INPUT_REPORT_USB			0x01
 #define DS4_INPUT_REPORT_USB_SIZE		64
 
+#define DS4_FEATURE_REPORT_FIRMWARE_INFO	0xa3
+#define DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE	49
 #define DS4_FEATURE_REPORT_PAIRING_INFO		0x12
 #define DS4_FEATURE_REPORT_PAIRING_INFO_SIZE	16
 
@@ -1494,6 +1496,30 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
 	return ERR_PTR(ret);
 }
 
+static int dualshock4_get_firmware_info(struct dualshock4 *ds4)
+{
+	uint8_t *buf;
+	int ret;
+
+	buf = kzalloc(DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_FIRMWARE_INFO, buf,
+			DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE);
+	if (ret) {
+		hid_err(ds4->base.hdev, "Failed to retrieve DualShock4 firmware info: %d\n", ret);
+		goto err_free;
+	}
+
+	ds4->base.hw_version = get_unaligned_le16(&buf[35]);
+	ds4->base.fw_version = get_unaligned_le16(&buf[41]);
+
+err_free:
+	kfree(buf);
+	return ret;
+}
+
 static int dualshock4_get_mac_address(struct dualshock4 *ds4)
 {
 	uint8_t *buf;
@@ -1600,6 +1626,12 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 	}
 	snprintf(hdev->uniq, sizeof(hdev->uniq), "%pMR", ds4->base.mac_address);
 
+	ret = dualshock4_get_firmware_info(ds4);
+	if (ret) {
+		hid_err(hdev, "Failed to get firmware info from DualShock4\n");
+		return ERR_PTR(ret);
+	}
+
 	ret = ps_devices_list_add(ps_dev);
 	if (ret)
 		return ERR_PTR(ret);
@@ -1616,6 +1648,12 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 		goto err;
 	}
 
+	/*
+	 * Reporting hardware and firmware is important as there are frequent updates, which
+	 * can change behavior.
+	 */
+	hid_info(hdev, "Registered DualShock4 controller hw_version=0x%08x fw_version=0x%08x\n",
+			ds4->base.hw_version, ds4->base.fw_version);
 	return &ds4->base;
 
 err:
-- 
2.37.3

[PATCH 03/13] HID: playstation: add DualShock4 battery support.

[Roderick Colenbrander]

Provide DualShock4 battery support through powersupply framework.

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

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index df4353a4f1e9..20111aa20e86 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -291,6 +291,12 @@ struct dualsense_output_report {
 #define DS4_FEATURE_REPORT_PAIRING_INFO		0x12
 #define DS4_FEATURE_REPORT_PAIRING_INFO_SIZE	16
 
+/* Status field of DualShock4 input report. */
+#define DS4_STATUS0_BATTERY_CAPACITY	GENMASK(3, 0)
+#define DS4_STATUS0_CABLE_STATE		BIT(4)
+/* Battery status within batery_status field. */
+#define DS4_BATTERY_STATUS_FULL		11
+
 struct dualshock4 {
 	struct ps_device base;
 	struct input_dev *gamepad;
@@ -302,15 +308,16 @@ struct dualshock4_input_report_common {
 	uint8_t rx, ry;
 	uint8_t buttons[3];
 	uint8_t z, rz;
-
 	uint8_t reserved[20];
+	uint8_t status[2];
+	uint8_t reserved2;
 } __packed;
-static_assert(sizeof(struct dualshock4_input_report_common) == 29);
+static_assert(sizeof(struct dualshock4_input_report_common) == 32);
 
 struct dualshock4_input_report_usb {
 	uint8_t report_id; /* 0x01 */
 	struct dualshock4_input_report_common common;
-	uint8_t reserved[34];
+	uint8_t reserved[31];
 } __packed;
 static_assert(sizeof(struct dualshock4_input_report_usb) == DS4_INPUT_REPORT_USB_SIZE);
 
@@ -1549,7 +1556,9 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	struct hid_device *hdev = ps_dev->hdev;
 	struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
 	struct dualshock4_input_report_common *ds4_report;
-	uint8_t value;
+	uint8_t battery_capacity, value;
+	int battery_status;
+	unsigned long flags;
 
 	/*
 	 * DualShock4 in USB uses the full HID report for reportID 1, but
@@ -1594,6 +1603,53 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	input_report_key(ds4->gamepad, BTN_MODE,   ds4_report->buttons[2] & DS_BUTTONS2_PS_HOME);
 	input_sync(ds4->gamepad);
 
+	/*
+	 * Interpretation of the battery_capacity data depends on the cable state.
+	 * When no cable is connected (bit4 is 0):
+	 * - 0:10: percentage in units of 10%.
+	 * When a cable is plugged in:
+	 * - 0-10: percentage in units of 10%.
+	 * - 11: battery is full
+	 * - 14: not charging due to Voltage or temperature error
+	 * - 15: charge error
+	 */
+	if (ds4_report->status[0] & DS4_STATUS0_CABLE_STATE) {
+		uint8_t battery_data = ds4_report->status[0] & DS4_STATUS0_BATTERY_CAPACITY;
+
+		if (battery_data < 10) {
+			/* Take the mid-point for each battery capacity value,
+			 * because on the hardware side 0 = 0-9%, 1=10-19%, etc.
+			 * This matches official platform behavior, which does
+			 * the same.
+			 */
+			battery_capacity = battery_data * 10 + 5;
+			battery_status = POWER_SUPPLY_STATUS_CHARGING;
+		} else if (battery_data == 10) {
+			battery_capacity = 100;
+			battery_status = POWER_SUPPLY_STATUS_CHARGING;
+		} else if (battery_data == DS4_BATTERY_STATUS_FULL) {
+			battery_capacity = 100;
+			battery_status = POWER_SUPPLY_STATUS_FULL;
+		} else { /* 14, 15 and undefined values */
+			battery_capacity = 0;
+			battery_status = POWER_SUPPLY_STATUS_UNKNOWN;
+		}
+	} else {
+		uint8_t battery_data = ds4_report->status[0] & DS4_STATUS0_BATTERY_CAPACITY;
+
+		if (battery_data < 10)
+			battery_capacity = battery_data * 10 + 5;
+		else /* 10 */
+			battery_capacity = 100;
+
+		battery_status = POWER_SUPPLY_STATUS_DISCHARGING;
+	}
+
+	spin_lock_irqsave(&ps_dev->lock, flags);
+	ps_dev->battery_capacity = battery_capacity;
+	ps_dev->battery_status = battery_status;
+	spin_unlock_irqrestore(&ps_dev->lock, flags);
+
 	return 0;
 }
 
@@ -1616,6 +1672,8 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 	ps_dev = &ds4->base;
 	ps_dev->hdev = hdev;
 	spin_lock_init(&ps_dev->lock);
+	ps_dev->battery_capacity = 100; /* initial value until parse_report. */
+	ps_dev->battery_status = POWER_SUPPLY_STATUS_UNKNOWN;
 	ps_dev->parse_report = dualshock4_parse_report;
 	hid_set_drvdata(hdev, ds4);
 
@@ -1642,6 +1700,10 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 		goto err;
 	}
 
+	ret = ps_device_register_battery(ps_dev);
+	if (ret)
+		goto err;
+
 	ret = ps_device_set_player_id(ps_dev);
 	if (ret) {
 		hid_err(hdev, "Failed to assign player id for DualShock4: %d\n", ret);
-- 
2.37.3

[PATCH 04/13] HID: playstation: add DualShock4 touchpad support.

[Roderick Colenbrander]

Support the DualShock4 touchpad as a separate input device. The code
describes the touchpad input reports through structures similar a bit
to the DualSense code.

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

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 20111aa20e86..63fc3a67ea74 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -291,17 +291,43 @@ struct dualsense_output_report {
 #define DS4_FEATURE_REPORT_PAIRING_INFO		0x12
 #define DS4_FEATURE_REPORT_PAIRING_INFO_SIZE	16
 
+/*
+ * Status of a DualShock4 touch point contact.
+ * Contact IDs, with highest bit set are 'inactive'
+ * and any associated data is then invalid.
+ */
+#define DS4_TOUCH_POINT_INACTIVE BIT(7)
+
 /* Status field of DualShock4 input report. */
 #define DS4_STATUS0_BATTERY_CAPACITY	GENMASK(3, 0)
 #define DS4_STATUS0_CABLE_STATE		BIT(4)
 /* Battery status within batery_status field. */
 #define DS4_BATTERY_STATUS_FULL		11
 
+/* DualShock4 hardware limits */
+#define DS4_TOUCHPAD_WIDTH	1920
+#define DS4_TOUCHPAD_HEIGHT	942
+
 struct dualshock4 {
 	struct ps_device base;
 	struct input_dev *gamepad;
+	struct input_dev *touchpad;
 };
 
+struct dualshock4_touch_point {
+	uint8_t contact;
+	uint8_t x_lo;
+	uint8_t x_hi:4, y_lo:4;
+	uint8_t y_hi;
+} __packed;
+static_assert(sizeof(struct dualshock4_touch_point) == 4);
+
+struct dualshock4_touch_report {
+	uint8_t timestamp;
+	struct dualshock4_touch_point points[2];
+} __packed;
+static_assert(sizeof(struct dualshock4_touch_report) == 9);
+
 /* Main DualShock4 input report excluding any BT/USB specific headers. */
 struct dualshock4_input_report_common {
 	uint8_t x, y;
@@ -317,7 +343,9 @@ static_assert(sizeof(struct dualshock4_input_report_common) == 32);
 struct dualshock4_input_report_usb {
 	uint8_t report_id; /* 0x01 */
 	struct dualshock4_input_report_common common;
-	uint8_t reserved[31];
+	uint8_t num_touch_reports;
+	struct dualshock4_touch_report touch_reports[3];
+	uint8_t reserved[3];
 } __packed;
 static_assert(sizeof(struct dualshock4_input_report_usb) == DS4_INPUT_REPORT_USB_SIZE);
 
@@ -1556,8 +1584,9 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	struct hid_device *hdev = ps_dev->hdev;
 	struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
 	struct dualshock4_input_report_common *ds4_report;
-	uint8_t battery_capacity, value;
-	int battery_status;
+	struct dualshock4_touch_report *touch_reports;
+	uint8_t battery_capacity, num_touch_reports, value;
+	int battery_status, i, j;
 	unsigned long flags;
 
 	/*
@@ -1570,6 +1599,8 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 		struct dualshock4_input_report_usb *usb = (struct dualshock4_input_report_usb *)data;
 
 		ds4_report = &usb->common;
+		num_touch_reports = usb->num_touch_reports;
+		touch_reports = usb->touch_reports;
 	} else {
 		hid_err(hdev, "Unhandled reportID=%d\n", report->id);
 		return -1;
@@ -1603,6 +1634,29 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	input_report_key(ds4->gamepad, BTN_MODE,   ds4_report->buttons[2] & DS_BUTTONS2_PS_HOME);
 	input_sync(ds4->gamepad);
 
+	for (i = 0; i < num_touch_reports; i++) {
+		struct dualshock4_touch_report *touch_report = &touch_reports[i];
+
+		for (j = 0; j < ARRAY_SIZE(touch_report->points); j++) {
+			struct dualshock4_touch_point *point = &touch_report->points[j];
+			bool active = (point->contact & DS4_TOUCH_POINT_INACTIVE) ? false : true;
+
+			input_mt_slot(ds4->touchpad, j);
+			input_mt_report_slot_state(ds4->touchpad, MT_TOOL_FINGER, active);
+
+			if (active) {
+				int x = (point->x_hi << 8) | point->x_lo;
+				int y = (point->y_hi << 4) | point->y_lo;
+
+				input_report_abs(ds4->touchpad, ABS_MT_POSITION_X, x);
+				input_report_abs(ds4->touchpad, ABS_MT_POSITION_Y, y);
+			}
+		}
+		input_mt_sync_frame(ds4->touchpad);
+		input_sync(ds4->touchpad);
+	}
+	input_report_key(ds4->touchpad, BTN_LEFT, ds4_report->buttons[2] & DS_BUTTONS2_TOUCHPAD);
+
 	/*
 	 * Interpretation of the battery_capacity data depends on the cable state.
 	 * When no cable is connected (bit4 is 0):
@@ -1700,6 +1754,12 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 		goto err;
 	}
 
+	ds4->touchpad = ps_touchpad_create(hdev, DS4_TOUCHPAD_WIDTH, DS4_TOUCHPAD_HEIGHT, 2);
+	if (IS_ERR(ds4->touchpad)) {
+		ret = PTR_ERR(ds4->touchpad);
+		goto err;
+	}
+
 	ret = ps_device_register_battery(ps_dev);
 	if (ret)
 		goto err;
-- 
2.37.3

[PATCH 05/13] HID: playstation: add DualShock4 accelerometer and gyroscope support.

[Roderick Colenbrander]

Support accelerometer and gyroscope as separate input devices similar
how DualSense and hid-sony do it.

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

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 63fc3a67ea74..03f33dea5d93 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -286,6 +286,8 @@ struct dualsense_output_report {
 #define DS4_INPUT_REPORT_USB			0x01
 #define DS4_INPUT_REPORT_USB_SIZE		64
 
+#define DS4_FEATURE_REPORT_CALIBRATION		0x02
+#define DS4_FEATURE_REPORT_CALIBRATION_SIZE	37
 #define DS4_FEATURE_REPORT_FIRMWARE_INFO	0xa3
 #define DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE	49
 #define DS4_FEATURE_REPORT_PAIRING_INFO		0x12
@@ -305,13 +307,27 @@ struct dualsense_output_report {
 #define DS4_BATTERY_STATUS_FULL		11
 
 /* DualShock4 hardware limits */
+#define DS4_ACC_RES_PER_G	8192
+#define DS4_ACC_RANGE		(4*DS_ACC_RES_PER_G)
+#define DS4_GYRO_RES_PER_DEG_S	1024
+#define DS4_GYRO_RANGE		(2048*DS_GYRO_RES_PER_DEG_S)
 #define DS4_TOUCHPAD_WIDTH	1920
 #define DS4_TOUCHPAD_HEIGHT	942
 
 struct dualshock4 {
 	struct ps_device base;
 	struct input_dev *gamepad;
+	struct input_dev *sensors;
 	struct input_dev *touchpad;
+
+	/* Calibration data for accelerometer and gyroscope. */
+	struct ps_calibration_data accel_calib_data[3];
+	struct ps_calibration_data gyro_calib_data[3];
+
+	/* Timestamp for sensor data */
+	bool sensor_timestamp_initialized;
+	uint32_t prev_sensor_timestamp;
+	uint32_t sensor_timestamp_us;
 };
 
 struct dualshock4_touch_point {
@@ -334,9 +350,16 @@ struct dualshock4_input_report_common {
 	uint8_t rx, ry;
 	uint8_t buttons[3];
 	uint8_t z, rz;
-	uint8_t reserved[20];
+
+	/* Motion sensors */
+	__le16 sensor_timestamp;
+	uint8_t sensor_temperature;
+	__le16 gyro[3]; /* x, y, z */
+	__le16 accel[3]; /* x, y, z */
+	uint8_t reserved2[5];
+
 	uint8_t status[2];
-	uint8_t reserved2;
+	uint8_t reserved3;
 } __packed;
 static_assert(sizeof(struct dualshock4_input_report_common) == 32);
 
@@ -1531,6 +1554,97 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
 	return ERR_PTR(ret);
 }
 
+static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
+{
+	struct hid_device *hdev = ds4->base.hdev;
+	short gyro_pitch_bias, gyro_pitch_plus, gyro_pitch_minus;
+	short gyro_yaw_bias, gyro_yaw_plus, gyro_yaw_minus;
+	short gyro_roll_bias, gyro_roll_plus, gyro_roll_minus;
+	short gyro_speed_plus, gyro_speed_minus;
+	short acc_x_plus, acc_x_minus;
+	short acc_y_plus, acc_y_minus;
+	short acc_z_plus, acc_z_minus;
+	int speed_2x;
+	int range_2g;
+	int ret = 0;
+	uint8_t *buf;
+
+	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);
+	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]);
+	gyro_speed_plus  = get_unaligned_le16(&buf[19]);
+	gyro_speed_minus = get_unaligned_le16(&buf[21]);
+	acc_x_plus       = get_unaligned_le16(&buf[23]);
+	acc_x_minus      = get_unaligned_le16(&buf[25]);
+	acc_y_plus       = get_unaligned_le16(&buf[27]);
+	acc_y_minus      = get_unaligned_le16(&buf[29]);
+	acc_z_plus       = get_unaligned_le16(&buf[31]);
+	acc_z_minus      = get_unaligned_le16(&buf[33]);
+
+	/*
+	 * Set gyroscope calibration and normalization parameters.
+	 * Data values will be normalized to 1/DS4_GYRO_RES_PER_DEG_S degree/s.
+	 */
+	speed_2x = (gyro_speed_plus + gyro_speed_minus);
+	ds4->gyro_calib_data[0].abs_code = ABS_RX;
+	ds4->gyro_calib_data[0].bias = gyro_pitch_bias;
+	ds4->gyro_calib_data[0].sens_numer = speed_2x*DS4_GYRO_RES_PER_DEG_S;
+	ds4->gyro_calib_data[0].sens_denom = gyro_pitch_plus - gyro_pitch_minus;
+
+	ds4->gyro_calib_data[1].abs_code = ABS_RY;
+	ds4->gyro_calib_data[1].bias = gyro_yaw_bias;
+	ds4->gyro_calib_data[1].sens_numer = speed_2x*DS4_GYRO_RES_PER_DEG_S;
+	ds4->gyro_calib_data[1].sens_denom = gyro_yaw_plus - gyro_yaw_minus;
+
+	ds4->gyro_calib_data[2].abs_code = ABS_RZ;
+	ds4->gyro_calib_data[2].bias = gyro_roll_bias;
+	ds4->gyro_calib_data[2].sens_numer = speed_2x*DS4_GYRO_RES_PER_DEG_S;
+	ds4->gyro_calib_data[2].sens_denom = gyro_roll_plus - gyro_roll_minus;
+
+	/*
+	 * Set accelerometer calibration and normalization parameters.
+	 * Data values will be normalized to 1/DS4_ACC_RES_PER_G g.
+	 */
+	range_2g = acc_x_plus - acc_x_minus;
+	ds4->accel_calib_data[0].abs_code = ABS_X;
+	ds4->accel_calib_data[0].bias = acc_x_plus - range_2g / 2;
+	ds4->accel_calib_data[0].sens_numer = 2*DS4_ACC_RES_PER_G;
+	ds4->accel_calib_data[0].sens_denom = range_2g;
+
+	range_2g = acc_y_plus - acc_y_minus;
+	ds4->accel_calib_data[1].abs_code = ABS_Y;
+	ds4->accel_calib_data[1].bias = acc_y_plus - range_2g / 2;
+	ds4->accel_calib_data[1].sens_numer = 2*DS4_ACC_RES_PER_G;
+	ds4->accel_calib_data[1].sens_denom = range_2g;
+
+	range_2g = acc_z_plus - acc_z_minus;
+	ds4->accel_calib_data[2].abs_code = ABS_Z;
+	ds4->accel_calib_data[2].bias = acc_z_plus - range_2g / 2;
+	ds4->accel_calib_data[2].sens_numer = 2*DS4_ACC_RES_PER_G;
+	ds4->accel_calib_data[2].sens_denom = range_2g;
+
+err_free:
+	kfree(buf);
+	return ret;
+}
+
 static int dualshock4_get_firmware_info(struct dualshock4 *ds4)
 {
 	uint8_t *buf;
@@ -1587,6 +1701,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	struct dualshock4_touch_report *touch_reports;
 	uint8_t battery_capacity, num_touch_reports, value;
 	int battery_status, i, j;
+	uint16_t sensor_timestamp;
 	unsigned long flags;
 
 	/*
@@ -1634,6 +1749,44 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	input_report_key(ds4->gamepad, BTN_MODE,   ds4_report->buttons[2] & DS_BUTTONS2_PS_HOME);
 	input_sync(ds4->gamepad);
 
+	/* Parse and calibrate gyroscope data. */
+	for (i = 0; i < ARRAY_SIZE(ds4_report->gyro); i++) {
+		int raw_data = (short)le16_to_cpu(ds4_report->gyro[i]);
+		int calib_data = mult_frac(ds4->gyro_calib_data[i].sens_numer,
+					   raw_data - ds4->gyro_calib_data[i].bias,
+					   ds4->gyro_calib_data[i].sens_denom);
+
+		input_report_abs(ds4->sensors, ds4->gyro_calib_data[i].abs_code, calib_data);
+	}
+
+	/* Parse and calibrate accelerometer data. */
+	for (i = 0; i < ARRAY_SIZE(ds4_report->accel); i++) {
+		int raw_data = (short)le16_to_cpu(ds4_report->accel[i]);
+		int calib_data = mult_frac(ds4->accel_calib_data[i].sens_numer,
+					   raw_data - ds4->accel_calib_data[i].bias,
+					   ds4->accel_calib_data[i].sens_denom);
+
+		input_report_abs(ds4->sensors, ds4->accel_calib_data[i].abs_code, calib_data);
+	}
+
+	/* Convert timestamp (in 5.33us unit) to timestamp_us */
+	sensor_timestamp = le16_to_cpu(ds4_report->sensor_timestamp);
+	if (!ds4->sensor_timestamp_initialized) {
+		ds4->sensor_timestamp_us = DIV_ROUND_CLOSEST(sensor_timestamp*16, 3);
+		ds4->sensor_timestamp_initialized = true;
+	} else {
+		uint16_t delta;
+
+		if (ds4->prev_sensor_timestamp > sensor_timestamp)
+			delta = (U16_MAX - ds4->prev_sensor_timestamp + sensor_timestamp + 1);
+		else
+			delta = sensor_timestamp - ds4->prev_sensor_timestamp;
+		ds4->sensor_timestamp_us += DIV_ROUND_CLOSEST(delta*16, 3);
+	}
+	ds4->prev_sensor_timestamp = sensor_timestamp;
+	input_event(ds4->sensors, EV_MSC, MSC_TIMESTAMP, ds4->sensor_timestamp_us);
+	input_sync(ds4->sensors);
+
 	for (i = 0; i < num_touch_reports; i++) {
 		struct dualshock4_touch_report *touch_report = &touch_reports[i];
 
@@ -1748,12 +1901,25 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 	if (ret)
 		return ERR_PTR(ret);
 
+	ret = dualshock4_get_calibration_data(ds4);
+	if (ret) {
+		hid_err(hdev, "Failed to get calibration data from DualShock4\n");
+		goto err;
+	}
+
 	ds4->gamepad = ps_gamepad_create(hdev, NULL);
 	if (IS_ERR(ds4->gamepad)) {
 		ret = PTR_ERR(ds4->gamepad);
 		goto err;
 	}
 
+	ds4->sensors = ps_sensors_create(hdev, DS4_ACC_RANGE, DS4_ACC_RES_PER_G,
+			DS4_GYRO_RANGE, DS4_GYRO_RES_PER_DEG_S);
+	if (IS_ERR(ds4->sensors)) {
+		ret = PTR_ERR(ds4->sensors);
+		goto err;
+	}
+
 	ds4->touchpad = ps_touchpad_create(hdev, DS4_TOUCHPAD_WIDTH, DS4_TOUCHPAD_HEIGHT, 2);
 	if (IS_ERR(ds4->touchpad)) {
 		ret = PTR_ERR(ds4->touchpad);
-- 
2.37.3

[PATCH 06/13] HID: playstation: Add DualShock4 rumble support.

[Roderick Colenbrander]

This patch implements DualShock4 rumble support in a similar manner
as the DualSense implementation. It adds an output worker with
granular control of different features of the main DualShock4 output
report.

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

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 03f33dea5d93..319f400dd946 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -285,6 +285,8 @@ struct dualsense_output_report {
 
 #define DS4_INPUT_REPORT_USB			0x01
 #define DS4_INPUT_REPORT_USB_SIZE		64
+#define DS4_OUTPUT_REPORT_USB			0x05
+#define DS4_OUTPUT_REPORT_USB_SIZE		32
 
 #define DS4_FEATURE_REPORT_CALIBRATION		0x02
 #define DS4_FEATURE_REPORT_CALIBRATION_SIZE	37
@@ -306,6 +308,9 @@ struct dualsense_output_report {
 /* Battery status within batery_status field. */
 #define DS4_BATTERY_STATUS_FULL		11
 
+/* Flags for DualShock4 output report. */
+#define DS4_OUTPUT_VALID_FLAG0_MOTOR		0x01
+
 /* DualShock4 hardware limits */
 #define DS4_ACC_RES_PER_G	8192
 #define DS4_ACC_RANGE		(4*DS_ACC_RES_PER_G)
@@ -328,6 +333,14 @@ struct dualshock4 {
 	bool sensor_timestamp_initialized;
 	uint32_t prev_sensor_timestamp;
 	uint32_t sensor_timestamp_us;
+
+	bool update_rumble;
+	uint8_t motor_left;
+	uint8_t motor_right;
+
+	struct work_struct output_worker;
+	bool output_worker_initialized;
+	void *output_report_dmabuf;
 };
 
 struct dualshock4_touch_point {
@@ -372,6 +385,45 @@ struct dualshock4_input_report_usb {
 } __packed;
 static_assert(sizeof(struct dualshock4_input_report_usb) == DS4_INPUT_REPORT_USB_SIZE);
 
+/* Common data between Bluetooth and USB DualShock4 output reports. */
+struct dualshock4_output_report_common {
+	uint8_t valid_flag0;
+	uint8_t valid_flag1;
+
+	uint8_t reserved;
+
+	uint8_t motor_right;
+	uint8_t motor_left;
+
+	uint8_t lightbar_red;
+	uint8_t lightbar_green;
+	uint8_t lightbar_blue;
+	uint8_t lightbar_blink_on;
+	uint8_t lightbar_blink_off;
+} __packed;
+
+struct dualshock4_output_report_usb {
+	uint8_t report_id; /* 0x5 */
+	struct dualshock4_output_report_common common;
+	uint8_t reserved[21];
+} __packed;
+static_assert(sizeof(struct dualshock4_output_report_usb) == DS4_OUTPUT_REPORT_USB_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.
+ * This structure hide the differences between the two to simplify sending output reports.
+ */
+struct dualshock4_output_report {
+	uint8_t *data; /* Start of data */
+	uint8_t len; /* Size of output report */
+
+	/* 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. */
+	struct dualshock4_output_report_common *common;
+};
+
 /*
  * Common gamepad buttons across DualShock 3 / 4 and DualSense.
  * Note: for device with a touchpad, touchpad button is not included
@@ -399,6 +451,7 @@ static const struct {int x; int y; } ps_gamepad_hat_mapping[] = {
 };
 
 static inline void dualsense_schedule_work(struct dualsense *ds);
+static inline void dualshock4_schedule_work(struct dualshock4 *ds4);
 static void dualsense_set_lightbar(struct dualsense *ds, uint8_t red, uint8_t green, uint8_t blue);
 
 /*
@@ -1692,6 +1745,49 @@ static int dualshock4_get_mac_address(struct dualshock4 *ds4)
 	return ret;
 }
 
+static void dualshock4_init_output_report(struct dualshock4 *ds4,
+		struct dualshock4_output_report *rp, void *buf)
+{
+	struct hid_device *hdev = ds4->base.hdev;
+
+	if (hdev->bus == BUS_USB) {
+		struct dualshock4_output_report_usb *usb = buf;
+
+		memset(usb, 0, sizeof(*usb));
+		usb->report_id = DS4_OUTPUT_REPORT_USB;
+
+		rp->data = buf;
+		rp->len = sizeof(*usb);
+		rp->usb = usb;
+		rp->common = &usb->common;
+	}
+}
+
+static void dualshock4_output_worker(struct work_struct *work)
+{
+	struct dualshock4 *ds4 = container_of(work, struct dualshock4, output_worker);
+	struct dualshock4_output_report report;
+	struct dualshock4_output_report_common *common;
+	unsigned long flags;
+
+	dualshock4_init_output_report(ds4, &report, ds4->output_report_dmabuf);
+	common = report.common;
+
+	spin_lock_irqsave(&ds4->base.lock, flags);
+
+	if (ds4->update_rumble) {
+		/* Select classic rumble style haptics and enable it. */
+		common->valid_flag0 |= DS4_OUTPUT_VALID_FLAG0_MOTOR;
+		common->motor_left = ds4->motor_left;
+		common->motor_right = ds4->motor_right;
+		ds4->update_rumble = false;
+	}
+
+	spin_unlock_irqrestore(&ds4->base.lock, flags);
+
+	hid_hw_output_report(ds4->base.hdev, report.data, report.len);
+}
+
 static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *report,
 		u8 *data, int size)
 {
@@ -1860,10 +1956,52 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	return 0;
 }
 
+static int dualshock4_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
+{
+	struct hid_device *hdev = input_get_drvdata(dev);
+	struct dualshock4 *ds4 = hid_get_drvdata(hdev);
+	unsigned long flags;
+
+	if (effect->type != FF_RUMBLE)
+		return 0;
+
+	spin_lock_irqsave(&ds4->base.lock, flags);
+	ds4->update_rumble = true;
+	ds4->motor_left = effect->u.rumble.strong_magnitude / 256;
+	ds4->motor_right = effect->u.rumble.weak_magnitude / 256;
+	spin_unlock_irqrestore(&ds4->base.lock, flags);
+
+	dualshock4_schedule_work(ds4);
+	return 0;
+}
+
+static void dualshock4_remove(struct ps_device *ps_dev)
+{
+	struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
+	unsigned long flags;
+
+	spin_lock_irqsave(&ds4->base.lock, flags);
+	ds4->output_worker_initialized = false;
+	spin_unlock_irqrestore(&ds4->base.lock, flags);
+
+	cancel_work_sync(&ds4->output_worker);
+}
+
+static inline void dualshock4_schedule_work(struct dualshock4 *ds4)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ds4->base.lock, flags);
+	if (ds4->output_worker_initialized)
+		schedule_work(&ds4->output_worker);
+	spin_unlock_irqrestore(&ds4->base.lock, flags);
+}
+
 static struct ps_device *dualshock4_create(struct hid_device *hdev)
 {
 	struct dualshock4 *ds4;
 	struct ps_device *ps_dev;
+	uint8_t max_output_report_size;
 	int ret;
 
 	ds4 = devm_kzalloc(&hdev->dev, sizeof(*ds4), GFP_KERNEL);
@@ -1882,8 +2020,16 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 	ps_dev->battery_capacity = 100; /* initial value until parse_report. */
 	ps_dev->battery_status = POWER_SUPPLY_STATUS_UNKNOWN;
 	ps_dev->parse_report = dualshock4_parse_report;
+	ps_dev->remove = dualshock4_remove;
+	INIT_WORK(&ds4->output_worker, dualshock4_output_worker);
+	ds4->output_worker_initialized = true;
 	hid_set_drvdata(hdev, ds4);
 
+	max_output_report_size = sizeof(struct dualshock4_output_report_usb);
+	ds4->output_report_dmabuf = devm_kzalloc(&hdev->dev, max_output_report_size, GFP_KERNEL);
+	if (!ds4->output_report_dmabuf)
+		return ERR_PTR(-ENOMEM);
+
 	ret = dualshock4_get_mac_address(ds4);
 	if (ret) {
 		hid_err(hdev, "Failed to get MAC address from DualShock4\n");
@@ -1907,7 +2053,7 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 		goto err;
 	}
 
-	ds4->gamepad = ps_gamepad_create(hdev, NULL);
+	ds4->gamepad = ps_gamepad_create(hdev, dualshock4_play_effect);
 	if (IS_ERR(ds4->gamepad)) {
 		ret = PTR_ERR(ds4->gamepad);
 		goto err;
-- 
2.37.3

[PATCH 07/13] HID: playstation: make LED brightness adjustable in ps_led_register.

[Roderick Colenbrander]

Make the max_brightness adjustable through ps_led_info struct. This
paves the way for a next DualShock4 patch to allow larger brightness
values.

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

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 319f400dd946..662c6f220571 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -60,6 +60,7 @@ struct ps_calibration_data {
 struct ps_led_info {
 	const char *name;
 	const char *color;
+	int max_brightness;
 	enum led_brightness (*brightness_get)(struct led_classdev *cdev);
 	int (*brightness_set)(struct led_classdev *cdev, enum led_brightness);
 };
@@ -703,7 +704,7 @@ static int ps_led_register(struct ps_device *ps_dev, struct led_classdev *led,
 		return -ENOMEM;
 
 	led->brightness = 0;
-	led->max_brightness = 1;
+	led->max_brightness = led_info->max_brightness;
 	led->flags = LED_CORE_SUSPENDRESUME;
 	led->brightness_get = led_info->brightness_get;
 	led->brightness_set_blocking = led_info->brightness_set;
@@ -1459,15 +1460,15 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
 	int i, ret;
 
 	static const struct ps_led_info player_leds_info[] = {
-		{ LED_FUNCTION_PLAYER1, "white", dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER1, "white", 1, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness },
-		{ LED_FUNCTION_PLAYER2, "white", dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER2, "white", 1, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness },
-		{ LED_FUNCTION_PLAYER3, "white", dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER3, "white", 1, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness },
-		{ LED_FUNCTION_PLAYER4, "white", dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER4, "white", 1, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness },
-		{ LED_FUNCTION_PLAYER5, "white", dualsense_player_led_get_brightness,
+		{ LED_FUNCTION_PLAYER5, "white", 1, dualsense_player_led_get_brightness,
 				dualsense_player_led_set_brightness }
 	};
 
-- 
2.37.3

[PATCH 08/13] HID: playstation: support DualShock4 lightbar.

[Roderick Colenbrander]

Expose the lightbar LEDs in the same manner as hid-sony through
individual LEDs for backwards compatibility reasons. There is a
slight change in LED naming to use the input device name as opposed
to the MAC address like hid-sony did. This is expected to not
cause any issues and should make the naming more compliant.

In addition set a default lightbar color based on player ID.

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

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 662c6f220571..d42fda13580a 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -311,6 +311,8 @@ struct dualsense_output_report {
 
 /* Flags for DualShock4 output report. */
 #define DS4_OUTPUT_VALID_FLAG0_MOTOR		0x01
+#define DS4_OUTPUT_VALID_FLAG0_LED		0x02
+#define DS4_OUTPUT_VALID_FLAG0_LED_BLINK	0x04
 
 /* DualShock4 hardware limits */
 #define DS4_ACC_RES_PER_G	8192
@@ -339,6 +341,14 @@ struct dualshock4 {
 	uint8_t motor_left;
 	uint8_t motor_right;
 
+	/* Lightbar leds */
+	bool update_lightbar;
+	bool lightbar_enabled; /* For use by global LED control. */
+	uint8_t lightbar_red;
+	uint8_t lightbar_green;
+	uint8_t lightbar_blue;
+	struct led_classdev lightbar_leds[4];
+
 	struct work_struct output_worker;
 	bool output_worker_initialized;
 	void *output_report_dmabuf;
@@ -697,8 +707,14 @@ static int ps_led_register(struct ps_device *ps_dev, struct led_classdev *led,
 {
 	int ret;
 
-	led->name = devm_kasprintf(&ps_dev->hdev->dev, GFP_KERNEL,
-			"%s:%s:%s", ps_dev->input_dev_name, led_info->color, led_info->name);
+	if (led_info->name) {
+		led->name = devm_kasprintf(&ps_dev->hdev->dev, GFP_KERNEL,
+				"%s:%s:%s", ps_dev->input_dev_name, led_info->color, led_info->name);
+	} else {
+		/* Backwards compatible mode for hid-sony, but not compliant with LED class spec. */
+		led->name = devm_kasprintf(&ps_dev->hdev->dev, GFP_KERNEL,
+				"%s:%s", ps_dev->input_dev_name, led_info->color);
+	}
 
 	if (!led->name)
 		return -ENOMEM;
@@ -1746,6 +1762,60 @@ static int dualshock4_get_mac_address(struct dualshock4 *ds4)
 	return ret;
 }
 
+static enum led_brightness dualshock4_led_get_brightness(struct led_classdev *led)
+{
+	struct hid_device *hdev = to_hid_device(led->dev->parent);
+	struct dualshock4 *ds4 = hid_get_drvdata(hdev);
+	unsigned int led_index;
+
+	led_index = led - ds4->lightbar_leds;
+	switch (led_index) {
+	case 0:
+		return ds4->lightbar_red;
+	case 1:
+		return ds4->lightbar_green;
+	case 2:
+		return ds4->lightbar_blue;
+	case 3:
+		return ds4->lightbar_enabled;
+	}
+
+	return -1;
+}
+
+static int dualshock4_led_set_brightness(struct led_classdev *led, enum led_brightness value)
+{
+	struct hid_device *hdev = to_hid_device(led->dev->parent);
+	struct dualshock4 *ds4 = hid_get_drvdata(hdev);
+	unsigned long flags;
+	unsigned int led_index;
+
+	spin_lock_irqsave(&ds4->base.lock, flags);
+
+	led_index = led - ds4->lightbar_leds;
+	switch (led_index) {
+	case 0:
+		ds4->lightbar_red = value;
+		break;
+	case 1:
+		ds4->lightbar_green = value;
+		break;
+	case 2:
+		ds4->lightbar_blue = value;
+		break;
+	case 3:
+		ds4->lightbar_enabled = !!value;
+	}
+
+	ds4->update_lightbar = true;
+
+	spin_unlock_irqrestore(&ds4->base.lock, flags);
+
+	dualshock4_schedule_work(ds4);
+
+	return 0;
+}
+
 static void dualshock4_init_output_report(struct dualshock4 *ds4,
 		struct dualshock4_output_report *rp, void *buf)
 {
@@ -1784,6 +1854,18 @@ static void dualshock4_output_worker(struct work_struct *work)
 		ds4->update_rumble = false;
 	}
 
+	if (ds4->update_lightbar) {
+		common->valid_flag0 |= DS4_OUTPUT_VALID_FLAG0_LED;
+		/* Comptabile behavior with hid-sony, which used a dummy global LED to
+		 * allow enabling/disabling the lightbar. The global LED maps to
+		 * lightbar_enabled.
+		 */
+		common->lightbar_red = ds4->lightbar_enabled ? ds4->lightbar_red : 0;
+		common->lightbar_green = ds4->lightbar_enabled ? ds4->lightbar_green : 0;
+		common->lightbar_blue = ds4->lightbar_enabled ? ds4->lightbar_blue : 0;
+		ds4->update_lightbar = false;
+	}
+
 	spin_unlock_irqrestore(&ds4->base.lock, flags);
 
 	hid_hw_output_report(ds4->base.hdev, report.data, report.len);
@@ -1998,12 +2080,52 @@ static inline void dualshock4_schedule_work(struct dualshock4 *ds4)
 	spin_unlock_irqrestore(&ds4->base.lock, flags);
 }
 
+/* Set default lightbar color based on player. */
+static void dualshock4_set_default_lightbar_colors(struct dualshock4 *ds4)
+{
+	/* Use same player colors as PlayStation 4.
+	 * Array of colors is in RGB.
+	 */
+	static const int player_colors[4][3] = {
+		{ 0x00, 0x00, 0x40 }, /* Blue */
+		{ 0x40, 0x00, 0x00 }, /* Red */
+		{ 0x00, 0x40, 0x00 }, /* Green */
+		{ 0x20, 0x00, 0x20 }  /* Pink */
+	};
+
+	uint8_t player_id = ds4->base.player_id % ARRAY_SIZE(player_colors);
+
+	ds4->lightbar_enabled = true;
+	ds4->lightbar_red = player_colors[player_id][0];
+	ds4->lightbar_green = player_colors[player_id][1];
+	ds4->lightbar_blue = player_colors[player_id][2];
+
+	ds4->update_lightbar = true;
+	dualshock4_schedule_work(ds4);
+}
+
 static struct ps_device *dualshock4_create(struct hid_device *hdev)
 {
 	struct dualshock4 *ds4;
 	struct ps_device *ps_dev;
 	uint8_t max_output_report_size;
-	int ret;
+	int i, ret;
+
+	/* The DualShock4 has an RGB lightbar, which the original hid-sony driver
+	 * exposed as a set of 4 LEDs for the 3 color channels and a global control.
+	 * Ideally this should have used the multi-color LED class, which didn't exist
+	 * yet. In addition the driver used a naming scheme not compliant with the LED
+	 * naming spec by using "<mac_address>:<color>", which contained many colons.
+	 * We use a more compliant by using "<device_name>:<color>" name now. Ideally
+	 * would have been "<device_name>:<color>:indicator", but that would break
+	 * existing applications (e.g. Android). Nothing matches against MAC address.
+	 */
+	static const struct ps_led_info lightbar_leds_info[] = {
+		{ NULL, "red", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness },
+		{ NULL, "green", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness },
+		{ NULL, "blue", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness },
+		{ NULL, "global", 1, dualshock4_led_get_brightness, dualshock4_led_set_brightness },
+	};
 
 	ds4 = devm_kzalloc(&hdev->dev, sizeof(*ds4), GFP_KERNEL);
 	if (!ds4)
@@ -2060,6 +2182,9 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 		goto err;
 	}
 
+	/* Use gamepad input device name as primary device name for e.g. LEDs */
+	ps_dev->input_dev_name = dev_name(&ds4->gamepad->dev);
+
 	ds4->sensors = ps_sensors_create(hdev, DS4_ACC_RANGE, DS4_ACC_RES_PER_G,
 			DS4_GYRO_RANGE, DS4_GYRO_RES_PER_DEG_S);
 	if (IS_ERR(ds4->sensors)) {
@@ -2077,12 +2202,22 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 	if (ret)
 		goto err;
 
+	for (i = 0; i < ARRAY_SIZE(lightbar_leds_info); i++) {
+		const struct ps_led_info *led_info = &lightbar_leds_info[i];
+
+		ret = ps_led_register(ps_dev, &ds4->lightbar_leds[i], led_info);
+		if (ret < 0)
+			goto err;
+	}
+
 	ret = ps_device_set_player_id(ps_dev);
 	if (ret) {
 		hid_err(hdev, "Failed to assign player id for DualShock4: %d\n", ret);
 		goto err;
 	}
 
+	dualshock4_set_default_lightbar_colors(ds4);
+
 	/*
 	 * Reporting hardware and firmware is important as there are frequent updates, which
 	 * can change behavior.
-- 
2.37.3

[PATCH 09/13] HID: playstation: support DualShock4 lightbar blink.

[Roderick Colenbrander]

Support lightbar blink through LEDs framework.

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

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index d42fda13580a..7ceb37f04d24 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -63,6 +63,7 @@ struct ps_led_info {
 	int max_brightness;
 	enum led_brightness (*brightness_get)(struct led_classdev *cdev);
 	int (*brightness_set)(struct led_classdev *cdev, enum led_brightness);
+	int (*blink_set)(struct led_classdev *led, unsigned long *on, unsigned long *off);
 };
 
 /* Seed values for DualShock4 / DualSense CRC32 for different report types. */
@@ -319,6 +320,7 @@ struct dualsense_output_report {
 #define DS4_ACC_RANGE		(4*DS_ACC_RES_PER_G)
 #define DS4_GYRO_RES_PER_DEG_S	1024
 #define DS4_GYRO_RANGE		(2048*DS_GYRO_RES_PER_DEG_S)
+#define DS4_LIGHTBAR_MAX_BLINK	255 /* 255 centiseconds */
 #define DS4_TOUCHPAD_WIDTH	1920
 #define DS4_TOUCHPAD_HEIGHT	942
 
@@ -343,10 +345,13 @@ struct dualshock4 {
 
 	/* Lightbar leds */
 	bool update_lightbar;
+	bool update_lightbar_blink;
 	bool lightbar_enabled; /* For use by global LED control. */
 	uint8_t lightbar_red;
 	uint8_t lightbar_green;
 	uint8_t lightbar_blue;
+	uint8_t lightbar_blink_on; /* In increments of 10ms. */
+	uint8_t lightbar_blink_off; /* In increments of 10ms. */
 	struct led_classdev lightbar_leds[4];
 
 	struct work_struct output_worker;
@@ -724,6 +729,7 @@ static int ps_led_register(struct ps_device *ps_dev, struct led_classdev *led,
 	led->flags = LED_CORE_SUSPENDRESUME;
 	led->brightness_get = led_info->brightness_get;
 	led->brightness_set_blocking = led_info->brightness_set;
+	led->blink_set = led_info->blink_set;
 
 	ret = devm_led_classdev_register(&ps_dev->hdev->dev, led);
 	if (ret) {
@@ -1783,6 +1789,37 @@ static enum led_brightness dualshock4_led_get_brightness(struct led_classdev *le
 	return -1;
 }
 
+static int dualshock4_led_set_blink(struct led_classdev *led, unsigned long *delay_on,
+		unsigned long *delay_off)
+{
+	struct hid_device *hdev = to_hid_device(led->dev->parent);
+	struct dualshock4 *ds4 = hid_get_drvdata(hdev);
+	unsigned long flags;
+
+	spin_lock_irqsave(&ds4->base.lock, flags);
+
+	if (!*delay_on && !*delay_off) {
+		/* Default to 1 Hz (50 centiseconds on, 50 centiseconds off). */
+		ds4->lightbar_blink_on = 50;
+		ds4->lightbar_blink_off = 50;
+	} else {
+		/* Blink delays in centiseconds. */
+		ds4->lightbar_blink_on = min_t(unsigned long, *delay_on/10, DS4_LIGHTBAR_MAX_BLINK);
+		ds4->lightbar_blink_off = min_t(unsigned long, *delay_off/10, DS4_LIGHTBAR_MAX_BLINK);
+	}
+
+	ds4->update_lightbar_blink = true;
+
+	spin_unlock_irqrestore(&ds4->base.lock, flags);
+
+	dualshock4_schedule_work(ds4);
+
+	*delay_on = ds4->lightbar_blink_on;
+	*delay_off = ds4->lightbar_blink_off;
+
+	return 0;
+}
+
 static int dualshock4_led_set_brightness(struct led_classdev *led, enum led_brightness value)
 {
 	struct hid_device *hdev = to_hid_device(led->dev->parent);
@@ -1866,6 +1903,13 @@ static void dualshock4_output_worker(struct work_struct *work)
 		ds4->update_lightbar = false;
 	}
 
+	if (ds4->update_lightbar_blink) {
+		common->valid_flag0 |= DS4_OUTPUT_VALID_FLAG0_LED_BLINK;
+		common->lightbar_blink_on = ds4->lightbar_blink_on;
+		common->lightbar_blink_off = ds4->lightbar_blink_off;
+		ds4->update_lightbar_blink = false;
+	}
+
 	spin_unlock_irqrestore(&ds4->base.lock, flags);
 
 	hid_hw_output_report(ds4->base.hdev, report.data, report.len);
@@ -2124,7 +2168,8 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 		{ NULL, "red", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness },
 		{ NULL, "green", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness },
 		{ NULL, "blue", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness },
-		{ NULL, "global", 1, dualshock4_led_get_brightness, dualshock4_led_set_brightness },
+		{ NULL, "global", 1, dualshock4_led_get_brightness, dualshock4_led_set_brightness,
+				dualshock4_led_set_blink },
 	};
 
 	ds4 = devm_kzalloc(&hdev->dev, sizeof(*ds4), GFP_KERNEL);
-- 
2.37.3

[PATCH 10/13] HID: playstation: add option to ignore CRC in ps_get_report.

[Roderick Colenbrander]

This patch adds a parameter to ps_get_report to ignore CRC checks.
This prepares for DualShock4, which has some HID reports, which lack CRC.

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

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 7ceb37f04d24..81a12aafed17 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -672,7 +672,8 @@ static struct input_dev *ps_gamepad_create(struct hid_device *hdev,
 	return gamepad;
 }
 
-static int ps_get_report(struct hid_device *hdev, uint8_t report_id, uint8_t *buf, size_t size)
+static int ps_get_report(struct hid_device *hdev, uint8_t report_id, uint8_t *buf, size_t size,
+		bool check_crc)
 {
 	int ret;
 
@@ -693,7 +694,7 @@ static int ps_get_report(struct hid_device *hdev, uint8_t report_id, uint8_t *bu
 		return -EINVAL;
 	}
 
-	if (hdev->bus == BUS_BLUETOOTH) {
+	if (hdev->bus == BUS_BLUETOOTH && check_crc) {
 		/* Last 4 bytes contains crc32. */
 		uint8_t crc_offset = size - 4;
 		uint32_t report_crc = get_unaligned_le32(&buf[crc_offset]);
@@ -894,7 +895,7 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
 		return -ENOMEM;
 
 	ret = ps_get_report(ds->base.hdev, DS_FEATURE_REPORT_CALIBRATION, buf,
-			DS_FEATURE_REPORT_CALIBRATION_SIZE);
+			DS_FEATURE_REPORT_CALIBRATION_SIZE, true);
 	if (ret) {
 		hid_err(ds->base.hdev, "Failed to retrieve DualSense calibration info: %d\n", ret);
 		goto err_free;
@@ -976,7 +977,7 @@ static int dualsense_get_firmware_info(struct dualsense *ds)
 		return -ENOMEM;
 
 	ret = ps_get_report(ds->base.hdev, DS_FEATURE_REPORT_FIRMWARE_INFO, buf,
-			DS_FEATURE_REPORT_FIRMWARE_INFO_SIZE);
+			DS_FEATURE_REPORT_FIRMWARE_INFO_SIZE, true);
 	if (ret) {
 		hid_err(ds->base.hdev, "Failed to retrieve DualSense firmware info: %d\n", ret);
 		goto err_free;
@@ -1009,7 +1010,7 @@ static int dualsense_get_mac_address(struct dualsense *ds)
 		return -ENOMEM;
 
 	ret = ps_get_report(ds->base.hdev, DS_FEATURE_REPORT_PAIRING_INFO, buf,
-			DS_FEATURE_REPORT_PAIRING_INFO_SIZE);
+			DS_FEATURE_REPORT_PAIRING_INFO_SIZE, true);
 	if (ret) {
 		hid_err(ds->base.hdev, "Failed to retrieve DualSense pairing info: %d\n", ret);
 		goto err_free;
@@ -1650,7 +1651,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 		return -ENOMEM;
 
 	ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION, buf,
-			DS4_FEATURE_REPORT_CALIBRATION_SIZE);
+			DS4_FEATURE_REPORT_CALIBRATION_SIZE, true);
 	if (ret) {
 		hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
 		goto err_free;
@@ -1731,7 +1732,7 @@ static int dualshock4_get_firmware_info(struct dualshock4 *ds4)
 		return -ENOMEM;
 
 	ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_FIRMWARE_INFO, buf,
-			DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE);
+			DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE, true);
 	if (ret) {
 		hid_err(ds4->base.hdev, "Failed to retrieve DualShock4 firmware info: %d\n", ret);
 		goto err_free;
@@ -1755,7 +1756,7 @@ static int dualshock4_get_mac_address(struct dualshock4 *ds4)
 		return -ENOMEM;
 
 	ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_PAIRING_INFO, buf,
-			DS4_FEATURE_REPORT_PAIRING_INFO_SIZE);
+			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;
-- 
2.37.3

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

[Roderick Colenbrander]

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(-)

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

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

[Benjamin Tissoires]

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
>

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

[Roderick Colenbrander]

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

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

[Roderick Colenbrander]

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

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

[Roderick Colenbrander]

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)

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

[Benjamin Tissoires]

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)
>

[PATCH 12/13] HID: playstation: set default DualShock4 BT poll interval to 4ms.

[Roderick Colenbrander]

The poll interval for DualShock4 in Bluetooth mode is adjustable
through the main output report. Configure it to 4ms, which is
similar to USB.

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

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index a2f1bd1400a2..05553d07cb1b 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -316,6 +316,17 @@ struct dualsense_output_report {
 /* Battery status within batery_status field. */
 #define DS4_BATTERY_STATUS_FULL		11
 
+/* The lower 6 bits of hw_control of the Bluetooth main output report
+ * control the interval at which Dualshock 4 reports data:
+ * 0x00 - 1ms
+ * 0x01 - 1ms
+ * 0x02 - 2ms
+ * 0x3E - 62ms
+ * 0x3F - disabled
+ */
+#define DS4_OUTPUT_HWCTL_BT_POLL_MASK	0x3F
+/* Default to 4ms poll interval, which is same as USB (not adjustable). */
+#define DS4_BT_DEFAULT_POLL_INTERVAL_MS	4
 #define DS4_OUTPUT_HWCTL_CRC32		0x40
 #define DS4_OUTPUT_HWCTL_HID		0x80
 
@@ -348,6 +359,10 @@ struct dualshock4 {
 	uint32_t prev_sensor_timestamp;
 	uint32_t sensor_timestamp_us;
 
+	/* Bluetooth poll interval */
+	bool update_bt_poll_interval;
+	uint8_t bt_poll_interval;
+
 	bool update_rumble;
 	uint8_t motor_left;
 	uint8_t motor_right;
@@ -2010,6 +2025,11 @@ static void dualshock4_output_worker(struct work_struct *work)
 		 */
 		report.bt->hw_control = DS4_OUTPUT_HWCTL_HID | DS4_OUTPUT_HWCTL_CRC32;
 
+		if (ds4->update_bt_poll_interval) {
+			report.bt->hw_control |= ds4->bt_poll_interval;
+			ds4->update_bt_poll_interval = false;
+		}
+
 		crc = crc32_le(0xFFFFFFFF, &seed, 1);
 		crc = ~crc32_le(crc, report.data, report.len - 4);
 
@@ -2241,6 +2261,13 @@ static inline void dualshock4_schedule_work(struct dualshock4 *ds4)
 	spin_unlock_irqrestore(&ds4->base.lock, flags);
 }
 
+static void dualshock4_set_bt_poll_interval(struct dualshock4 *ds4, uint8_t interval)
+{
+	ds4->bt_poll_interval = interval;
+	ds4->update_bt_poll_interval = true;
+	dualshock4_schedule_work(ds4);
+}
+
 /* Set default lightbar color based on player. */
 static void dualshock4_set_default_lightbar_colors(struct dualshock4 *ds4)
 {
@@ -2372,6 +2399,8 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 			goto err;
 	}
 
+	dualshock4_set_bt_poll_interval(ds4, DS4_BT_DEFAULT_POLL_INTERVAL_MS);
+
 	ret = ps_device_set_player_id(ps_dev);
 	if (ret) {
 		hid_err(hdev, "Failed to assign player id for DualShock4: %d\n", ret);
-- 
2.37.3

[PATCH 13/13] HID: playstation: add DualShock4 dongle support.

[Roderick Colenbrander]

This patch adds support for the DualShock4 dongle in a very similar
way we contributed to hid-sony before.

The dongle is a USB to Bluetooth bridge and uses the same HID reports
as a USB device. It reports data through the DS4's main USB input
report independent on whether a Bluetooth controller is connected.
For this reason there is custom dongle report parsing code to
detect controller hotplug and kick of calibration work until we
are ready to process actual input reports.

The logic also incorporates a workaround needed for Steam in which
hid-playstation and Steam using hidraw can fight.

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

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 05553d07cb1b..bae3e712a562 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -315,6 +315,11 @@ struct dualsense_output_report {
 #define DS4_STATUS0_CABLE_STATE		BIT(4)
 /* Battery status within batery_status field. */
 #define DS4_BATTERY_STATUS_FULL		11
+/* Status1 bit2 contains dongle connection state:
+ * 0 = connectd
+ * 1 = disconnected
+ */
+#define DS4_STATUS1_DONGLE_STATE	BIT(2)
 
 /* The lower 6 bits of hw_control of the Bluetooth main output report
  * control the interval at which Dualshock 4 reports data:
@@ -344,6 +349,13 @@ struct dualsense_output_report {
 #define DS4_TOUCHPAD_WIDTH	1920
 #define DS4_TOUCHPAD_HEIGHT	942
 
+enum dualshock4_dongle_state {
+	DONGLE_DISCONNECTED,
+	DONGLE_CALIBRATING,
+	DONGLE_CONNECTED,
+	DONGLE_DISABLED
+};
+
 struct dualshock4 {
 	struct ps_device base;
 	struct input_dev *gamepad;
@@ -354,6 +366,11 @@ struct dualshock4 {
 	struct ps_calibration_data accel_calib_data[3];
 	struct ps_calibration_data gyro_calib_data[3];
 
+	/* Only used on dongle to track state transitions. */
+	enum dualshock4_dongle_state dongle_state;
+	/* Used during calibration. */
+	struct work_struct dongle_hotplug_worker;
+
 	/* Timestamp for sensor data */
 	bool sensor_timestamp_initialized;
 	uint32_t prev_sensor_timestamp;
@@ -513,9 +530,11 @@ static const struct {int x; int y; } ps_gamepad_hat_mapping[] = {
 	{0, 0},
 };
 
+static int dualshock4_get_calibration_data(struct dualshock4 *ds4);
 static inline void dualsense_schedule_work(struct dualsense *ds);
 static inline void dualshock4_schedule_work(struct dualshock4 *ds4);
 static void dualsense_set_lightbar(struct dualsense *ds, uint8_t red, uint8_t green, uint8_t blue);
+static void dualshock4_set_default_lightbar_colors(struct dualshock4 *ds4);
 
 /*
  * Add a new ps_device to ps_devices if it doesn't exist.
@@ -1678,6 +1697,33 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
 	return ERR_PTR(ret);
 }
 
+static void dualshock4_dongle_calibration_work(struct work_struct *work)
+{
+	struct dualshock4 *ds4 = container_of(work, struct dualshock4, dongle_hotplug_worker);
+	unsigned long flags;
+	enum dualshock4_dongle_state dongle_state;
+	int ret;
+
+	ret = dualshock4_get_calibration_data(ds4);
+	if (ret < 0) {
+		/* This call is very unlikely to fail for the dongle. When it
+		 * fails we are probably in a very bad state, so mark the
+		 * dongle as disabled. We will re-enable the dongle if a new
+		 * DS4 hotplug is detect from sony_raw_event as any issues
+		 * are likely resolved then (the dongle is quite stupid).
+		 */
+		hid_err(ds4->base.hdev, "DualShock 4 USB dongle: calibration failed, disabling device\n");
+		dongle_state = DONGLE_DISABLED;
+	} else {
+		hid_info(ds4->base.hdev, "DualShock 4 USB dongle: calibration completed\n");
+		dongle_state = DONGLE_CONNECTED;
+	}
+
+	spin_lock_irqsave(&ds4->base.lock, flags);
+	ds4->dongle_state = dongle_state;
+	spin_unlock_irqrestore(&ds4->base.lock, flags);
+}
+
 static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 {
 	struct hid_device *hdev = ds4->base.hdev;
@@ -1694,15 +1740,34 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
 	uint8_t *buf;
 
 	if (ds4->base.hdev->bus == BUS_USB) {
+		int retries;
+
 		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;
+		/* We should normally receive the feature report data we asked
+		 * for, but hidraw applications such as Steam can issue feature
+		 * reports as well. In particular for Dongle reconnects, Steam
+		 * and this function are competing resulting in often receiving
+		 * data for a different HID report, so retry a few times.
+		 */
+		for (retries = 0; retries < 3; retries++) {
+			ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION, buf,
+					DS4_FEATURE_REPORT_CALIBRATION_SIZE, true);
+			if (ret) {
+				if (retries < 2) {
+					hid_warn(hdev, "Retrying DualShock 4 get calibration report (0x02) request\n");
+					continue;
+				} else {
+					ret = -EILSEQ;
+					goto err_free;
+				}
+				hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
+				goto err_free;
+			} else {
+				break;
+			}
 		}
 	} else { /* Bluetooth */
 		buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE, GFP_KERNEL);
@@ -2220,6 +2285,62 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 	return 0;
 }
 
+static int dualshock4_dongle_parse_report(struct ps_device *ps_dev, struct hid_report *report,
+		u8 *data, int size)
+{
+	struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
+	bool connected = false;
+
+	/* The dongle reports data using the main USB report (0x1) no matter whether a controller
+	 * is connected with mostly zeros. The report does contain dongle status, which we use to
+	 * determine if a controller is connected and if so we forward to the regular DualShock4
+	 * parsing code.
+	 */
+	if (data[0] == DS4_INPUT_REPORT_USB && size == DS4_INPUT_REPORT_USB_SIZE) {
+		struct dualshock4_input_report_common *ds4_report = (struct dualshock4_input_report_common *)&data[1];
+		unsigned long flags;
+
+		connected = ds4_report->status[1] & DS4_STATUS1_DONGLE_STATE ? false : true;
+
+		if (ds4->dongle_state == DONGLE_DISCONNECTED && connected) {
+			hid_info(ps_dev->hdev, "DualShock 4 USB dongle: controller connected\n");
+
+			dualshock4_set_default_lightbar_colors(ds4);
+
+			spin_lock_irqsave(&ps_dev->lock, flags);
+			ds4->dongle_state = DONGLE_CALIBRATING;
+			spin_unlock_irqrestore(&ps_dev->lock, flags);
+
+			schedule_work(&ds4->dongle_hotplug_worker);
+
+			/* Don't process the report since we don't have
+			 * calibration data, but let hidraw have it anyway.
+			 */
+			return 0;
+		} else if ((ds4->dongle_state == DONGLE_CONNECTED ||
+			    ds4->dongle_state == DONGLE_DISABLED) && !connected) {
+			hid_info(ps_dev->hdev, "DualShock 4 USB dongle: controller disconnected\n");
+
+			spin_lock_irqsave(&ps_dev->lock, flags);
+			ds4->dongle_state = DONGLE_DISCONNECTED;
+			spin_unlock_irqrestore(&ps_dev->lock, flags);
+
+			/* Return 0, so hidraw can get the report. */
+			return 0;
+		} else if (ds4->dongle_state == DONGLE_CALIBRATING ||
+			   ds4->dongle_state == DONGLE_DISABLED ||
+			   ds4->dongle_state == DONGLE_DISCONNECTED) {
+			/* Return 0, so hidraw can get the report. */
+			return 0;
+		}
+	}
+
+	if (connected)
+		return dualshock4_parse_report(ps_dev, report, data, size);
+
+	return 0;
+}
+
 static int dualshock4_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
 {
 	struct hid_device *hdev = input_get_drvdata(dev);
@@ -2249,6 +2370,9 @@ static void dualshock4_remove(struct ps_device *ps_dev)
 	spin_unlock_irqrestore(&ds4->base.lock, flags);
 
 	cancel_work_sync(&ds4->output_worker);
+
+	if (ps_dev->hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE)
+		cancel_work_sync(&ds4->dongle_hotplug_worker);
 }
 
 static inline void dualshock4_schedule_work(struct dualshock4 *ds4)
@@ -2342,6 +2466,14 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
 	if (!ds4->output_report_dmabuf)
 		return ERR_PTR(-ENOMEM);
 
+	if (hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE) {
+		ds4->dongle_state = DONGLE_DISCONNECTED;
+		INIT_WORK(&ds4->dongle_hotplug_worker, dualshock4_dongle_calibration_work);
+
+		/* Override parse report for dongle specific hotplug handling. */
+		ps_dev->parse_report = dualshock4_dongle_parse_report;
+	}
+
 	ret = dualshock4_get_mac_address(ds4);
 	if (ret) {
 		hid_err(hdev, "Failed to get MAC address from DualShock4\n");
@@ -2457,7 +2589,8 @@ static int ps_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	if (hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER ||
-		hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) {
+		hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_2 ||
+		hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE) {
 		dev = dualshock4_create(hdev);
 		if (IS_ERR(dev)) {
 			hid_err(hdev, "Failed to create dualshock4.\n");
@@ -2503,6 +2636,7 @@ static const struct hid_device_id ps_devices[] = {
 	{ 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) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE) },
 	/* Sony DualSense controllers for PS5 */
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) },
-- 
2.37.3

Re: [PATCH 00/13] hid: playstation: add DualShock4 support

[Jiri Kosina]

On Sat, 29 Oct 2022, Roderick Colenbrander wrote:

> Hi,
> 
> Last year, we introduced hid-playstation as the start of a new driver
> for officially supported PlayStation peripherals. The driver initially
> only supported DualSense devices, but we promised to also bring support
> for at least DualShock4 as well.
> 
> This patch series adds DualShock4 support to hid-playstation. It should
> be considered a brand new driver written from scratch in the same design
> as hid-playstation. The driver is more documented and uses structures
> instead of byte offsets with magical values. The driver should be more
> clear and easier to follow. A few little sections of code cary over, which
> Sony contributed before for sensor calibration or dongle support.
> 
> Functionality wise the driver is equivalent to hid-sony. The only subtle
> change is in the naming of the lightbar LEDs. Previously they used a naming
> scheme like '<mac_address>:<color>", which doesn't follow the LED class standards.
> The new scheme uses '<device_name>:<color>' (e.g. input10:red), which is more
> compliant. Due to backwards compatibility in particular for Android, we couldn't
> make it fully compliant. Nor were we able to use the new multicolor LED class.
> 
> Aside from the LED code, the other features behave the same way. The hid-tools
> tests all pass as well. One small change is that we use a different HID report
> 0x12 to get the MAC address in USB mode. This report is the official one even
> though other ones can get the info too, but e.g. clone devices don't tend to
> support it.

Hi Roderick,

this is now queued in hid.git#for-6.2/sony.

Thanks for all the work on this,

-- 
Jiri Kosina
SUSE Labs